Thread: BUG #18896: A potential problem in heap_page_items (pageinspect, PG-17)

The following bug has been logged on the website:

Bug reference:      18896
Logged by:          Dmitry Kovalenko
Email address:      d.kovalenko@postgrespro.ru
PostgreSQL version: 17.4
Operating system:   Ubuntu 2024.04
Description:

Hello,

Please, look at this code in heap_page_items function:


https://github.com/postgres/postgres/blob/11ff192b5bb707ba9ec13a0b6c7468874403abb3/contrib/pageinspect/heapfuncs.c#L231-L248

If lp_len value is less than tuphdr->t_hoff, we get a problem with negative
value in tuple_data_len

I think, we should move the code of /* Copy raw tuple data into bytea
attribute */ into the next if (.... tuphdr->t_hoff <= lp_len ....)
section.

And set nulls[13] = true; to else section here -

https://github.com/postgres/postgres/blob/11ff192b5bb707ba9ec13a0b6c7468874403abb3/contrib/pageinspect/heapfuncs.c#L267C5-L268

-------- [copy of problem block at lines 231 ... 248]
    /* Copy raw tuple data into bytea attribute */
    tuple_data_len = lp_len - tuphdr->t_hoff;  // < -----------------
SUBSTRACT
    tuple_data_bytea = (bytea *) palloc(tuple_data_len + VARHDRSZ);
    SET_VARSIZE(tuple_data_bytea, tuple_data_len + VARHDRSZ);
    memcpy(VARDATA(tuple_data_bytea), (char *) tuphdr + tuphdr->t_hoff,
        tuple_data_len); // < ------------------- USAGE
    values[13] = PointerGetDatum(tuple_data_bytea);


    /*
     * We already checked that the item is completely within the raw
     * page passed to us, with the length given in the line pointer.
     * Let's check that t_hoff doesn't point over lp_len, before using
     * it to access t_bits and oid.
     */
    if (tuphdr->t_hoff >= SizeofHeapTupleHeader &&
        tuphdr->t_hoff <= lp_len && // < ------------------ VERIFICATION
        tuphdr->t_hoff == MAXALIGN(tuphdr->t_hoff))
    {
--------

I hope, this problem can be fixed really easily.

Thanks&Regards,
Kovalenko Dmitry
PostgresPro, Russia.

PG Bug reporting form <noreply@postgresql.org> writes:
> Please, look at this code in heap_page_items function:
>
https://github.com/postgres/postgres/blob/11ff192b5bb707ba9ec13a0b6c7468874403abb3/contrib/pageinspect/heapfuncs.c#L231-L248
> If lp_len value is less than tuphdr->t_hoff, we get a problem with negative
> value in tuple_data_len

Yeah, you're right.

> I think, we should move the code of /* Copy raw tuple data into bytea
> attribute */ into the next if (.... tuphdr->t_hoff <= lp_len ....)
> section.

Agreed, that looks like a good fix.  I'll make it so.

            regards, tom lane