projects / postgresql.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 88e9471)
Be more wary of corrupt data in pageinspect's heap_page_items().
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 19 Apr 2025 20:37:42 +0000 (16:37 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 19 Apr 2025 20:37:42 +0000 (16:37 -0400)
The original intent in heap_page_items() was to return nulls, not
throw an error or crash, if an item was sufficiently corrupt that
we couldn't safely extract data from it.  However, commit d6061f83a
utterly missed that memo, and not only put in an un-length-checked
copy of the tuple's data section, but also managed to break the check
on sane nulls-bitmap length.  Either mistake could possibly lead to
a SIGSEGV crash if the tuple is corrupt.

Bug: #18896
Reported-by: Dmitry Kovalenko <d.kovalenko@postgrespro.ru>
Author: Dmitry Kovalenko <d.kovalenko@postgrespro.ru>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/18896-add267b8e06663e3@postgresql.org
Backpatch-through: 13

contrib/pageinspect/heapfuncs.c patch | blob | blame | history

diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index aa8e1bd6df4a63ad1aa534b2a3839fae97a419e8..377ae30d1fa711a226c3160177f4b06b1c211a74 100644 (file)
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -212,11 +212,8 @@ heap_page_items(PG_FUNCTION_ARGS)
            lp_offset + lp_len <= raw_page_size)
        {
            HeapTupleHeader tuphdr;
-           bytea      *tuple_data_bytea;
-           int         tuple_data_len;
 
            /* Extract information from the tuple header */
-
            tuphdr = (HeapTupleHeader) PageGetItem(page, id);
 
            values[4] = UInt32GetDatum(HeapTupleHeaderGetRawXmin(tuphdr));
@@ -228,31 +225,32 @@ heap_page_items(PG_FUNCTION_ARGS)
            values[9] = UInt32GetDatum(tuphdr->t_infomask);
            values[10] = UInt8GetDatum(tuphdr->t_hoff);
 
-           /* Copy raw tuple data into bytea attribute */
-           tuple_data_len = lp_len - tuphdr->t_hoff;
-           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);
-           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.
+            * But t_hoff could be out of range, so check it before relying on
+            * it to fetch additional info.
             */
            if (tuphdr->t_hoff >= SizeofHeapTupleHeader &&
                tuphdr->t_hoff <= lp_len &&
                tuphdr->t_hoff == MAXALIGN(tuphdr->t_hoff))
            {
+               int         tuple_data_len;
+               bytea      *tuple_data_bytea;
+
+               /* Copy null bitmask and OID, if present */
                if (tuphdr->t_infomask & HEAP_HASNULL)
                {
-                   int         bits_len;
-
-                   bits_len =
-                       BITMAPLEN(HeapTupleHeaderGetNatts(tuphdr)) * BITS_PER_BYTE;
-                   values[11] = CStringGetTextDatum(bits_to_text(tuphdr->t_bits, bits_len));
+                   int         bitmaplen;
+
+                   bitmaplen = BITMAPLEN(HeapTupleHeaderGetNatts(tuphdr));
+                   /* better range-check the attribute count, too */
+                   if (bitmaplen <= tuphdr->t_hoff - SizeofHeapTupleHeader)
+                       values[11] =
+                           CStringGetTextDatum(bits_to_text(tuphdr->t_bits,
+                                                            bitmaplen * BITS_PER_BYTE));
+                   else
+                       nulls[11] = true;
                }
                else
                    nulls[11] = true;
@@ -261,11 +259,22 @@ heap_page_items(PG_FUNCTION_ARGS)
                    values[12] = HeapTupleHeaderGetOidOld(tuphdr);
                else
                    nulls[12] = true;
+
+               /* Copy raw tuple data into bytea attribute */
+               tuple_data_len = lp_len - tuphdr->t_hoff;
+               tuple_data_bytea = (bytea *) palloc(tuple_data_len + VARHDRSZ);
+               SET_VARSIZE(tuple_data_bytea, tuple_data_len + VARHDRSZ);
+               if (tuple_data_len > 0)
+                   memcpy(VARDATA(tuple_data_bytea),
+                          (char *) tuphdr + tuphdr->t_hoff,
+                          tuple_data_len);
+               values[13] = PointerGetDatum(tuple_data_bytea);
            }
            else
            {
                nulls[11] = true;
                nulls[12] = true;
+               nulls[13] = true;
            }
        }
        else
This is the main PostgreSQL git repository.