Be more wary of corrupt data in pageinspect's heap_page_items().

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

diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 07c3cee5288213b63a540d9dd620fd73bfa56fcc..b067dd3660e7b189b9eef5b82a5abd7c4bb82c5c 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