From 422482f339bd0aff0808e5c22f14eb74088c6312 Mon Sep 17 00:00:00 2001 From: shihao zhong Date: Wed, 21 Oct 2026 23:27:47 -0400 Subject: [PATCH v4 2/2] pageinspect: validate page headers, and BRIN, GIN and GiST page contents Walking a page whose header is corrupt could address memory outside the page image. Verify the header first. Also check that BRIN line pointers, the stored values of BRIN and GiST tuples, and GIN posting lists stay within their bounds, as for the other access methods. Reported-by: Yilin Zhang --- contrib/pageinspect/brinfuncs.c | 71 +++++++++++++++++++++ contrib/pageinspect/btreefuncs.c | 1 + contrib/pageinspect/expected/brin.out | 19 ++++++ contrib/pageinspect/expected/gin.out | 7 +++ contrib/pageinspect/expected/gist.out | 10 +++ contrib/pageinspect/expected/page.out | 6 ++ contrib/pageinspect/ginfuncs.c | 13 +++- contrib/pageinspect/gistfuncs.c | 39 ++++++++++++ contrib/pageinspect/hashfuncs.c | 2 + contrib/pageinspect/heapfuncs.c | 1 + contrib/pageinspect/pageinspect.h | 3 + contrib/pageinspect/rawpage.c | 88 +++++++++++++++++++++++++++ contrib/pageinspect/sql/brin.sql | 16 +++++ contrib/pageinspect/sql/gin.sql | 7 +++ contrib/pageinspect/sql/gist.sql | 9 +++ contrib/pageinspect/sql/page.sql | 6 ++ 16 files changed, 297 insertions(+), 1 deletion(-) diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c index c64124b5b02..33300af6dcd 100644 --- a/contrib/pageinspect/brinfuncs.c +++ b/contrib/pageinspect/brinfuncs.c @@ -23,6 +23,7 @@ #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/tuplestore.h" +#include "utils/typcache.h" PG_FUNCTION_INFO_V1(brin_page_type); PG_FUNCTION_INFO_V1(brin_page_items); @@ -95,6 +96,8 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype) { Page page = get_page_from_raw(raw_page); + verify_page_header(page); + if (PageIsNew(page)) return page; @@ -118,6 +121,62 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype) return page; } +/* + * Check that the header, null bitmap and stored values of a BRIN tuple all + * lie within the "len" bytes that its line pointer claims. brin_deform_tuple() + * trusts the tuple, so we must not call it on one that reaches past the end. + * + * The contents of a value are not checked. + */ +static void +verify_brin_tuple(BrinDesc *bdesc, BrinTuple *tuple, Size len, + OffsetNumber offset) +{ + int natts = bdesc->bd_tupdesc->natts; + bool hasnulls = BrinTupleHasNulls(tuple); + Size hoff = BrinTupleDataOffset(tuple); + Size minhoff = SizeOfBrinTuple; + uint8 *nullbits = (uint8 *) ((char *) tuple + SizeOfBrinTuple); + char *data; + Size avail; + Size off = 0; + + if (hasnulls) + minhoff += BITMAPLEN(natts * 2); + + if (hoff < minhoff || hoff > len) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("invalid tuple at offset %u in BRIN page", offset), + errdetail("Data offset %zu is outside the %zu-byte tuple.", + hoff, len))); + + data = (char *) tuple + hoff; + avail = len - hoff; + + for (int attno = 0; attno < natts; attno++) + { + BrinOpcInfo *opcinfo = bdesc->bd_info[attno]; + + /* all-nulls columns store no values */ + if (hasnulls && !att_isnull(attno, nullbits)) + continue; + + for (int i = 0; i < opcinfo->oi_nstored; i++) + { + TypeCacheEntry *typ = opcinfo->oi_typcache[i]; + + if (!stored_value_fits(data, avail, &off, + typ->typlen, typ->typalign)) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("invalid tuple at offset %u in BRIN page", offset), + errdetail("Stored values extend past the end of the %zu-byte tuple.", + len))); + } + } +} + /* Number of output arguments (columns) for brin_page_items() */ #define BRIN_PAGE_ITEMS_V1_12 8 @@ -232,6 +291,18 @@ brin_page_items(PG_FUNCTION_ARGS) itemId = PageGetItemId(page, offset); if (ItemIdIsUsed(itemId)) { + /* Check that the line pointer and tuple lie within the page. */ + if (ItemIdGetOffset(itemId) != MAXALIGN(ItemIdGetOffset(itemId)) || + ItemIdGetLength(itemId) < SizeOfBrinTuple || + ItemIdGetOffset(itemId) + ItemIdGetLength(itemId) > BLCKSZ) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid line pointer at offset %u in BRIN page", + offset))); + + verify_brin_tuple(bdesc, (BrinTuple *) PageGetItem(page, itemId), + ItemIdGetLength(itemId), offset); + dtup = brin_deform_tuple(bdesc, (BrinTuple *) PageGetItem(page, itemId), NULL); diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c index 5fc7dc993f7..238b06fcec4 100644 --- a/contrib/pageinspect/btreefuncs.c +++ b/contrib/pageinspect/btreefuncs.c @@ -787,6 +787,7 @@ bt_page_items_bytea(PG_FUNCTION_ARGS) uargs = palloc_object(ua_page_items); uargs->page = get_page_from_raw(raw_page); + verify_page_header(uargs->page); if (PageIsNew(uargs->page)) { diff --git a/contrib/pageinspect/expected/brin.out b/contrib/pageinspect/expected/brin.out index b6a43bbb01b..f41f3e986a6 100644 --- a/contrib/pageinspect/expected/brin.out +++ b/contrib/pageinspect/expected/brin.out @@ -106,6 +106,25 @@ SELECT (COUNT(*) = (SELECT relpages FROM pg_class WHERE relname = 'test2')) AS r t (1 row) +-- A corrupt line pointer must be reported, not read out of bounds. All-ones is +-- an invalid (out-of-range, unaligned) line pointer on any architecture. +SELECT brin_page_items(set_byte(set_byte(set_byte(set_byte(get_raw_page('test1_a_idx', 2), 24, 255), 25, 255), 26, 255), 27, 255), 'test1_a_idx'); +ERROR: invalid line pointer at offset 1 in BRIN page +-- A tuple must not reach past the length given by its line pointer. First +-- set the tuple's data offset (the low bits of bt_info) beyond the tuple, then +-- make the first stored value of a text index claim a length beyond the tuple. +-- The tuple is the only one on the page, so it starts at pd_upper. +SELECT brin_page_items(set_byte(get_raw_page('test1_a_idx', 2), + (SELECT upper FROM page_header(get_raw_page('test1_a_idx', 2))) + 4, 31), + 'test1_a_idx'); +ERROR: invalid tuple at offset 1 in BRIN page +DETAIL: Data offset 31 is outside the 16-byte tuple. +CREATE INDEX test1_b_idx ON test1 USING brin (b); +SELECT brin_page_items(set_byte(get_raw_page('test1_b_idx', 2), + (SELECT upper FROM page_header(get_raw_page('test1_b_idx', 2))) + 8, 255), + 'test1_b_idx'); +ERROR: invalid tuple at offset 1 in BRIN page +DETAIL: Stored values extend past the end of the 16-byte tuple. DROP TABLE test1; DROP TABLE test2; -- Test that parallel index build produces the same BRIN index as serial build. diff --git a/contrib/pageinspect/expected/gin.out b/contrib/pageinspect/expected/gin.out index ff1da6a5a17..dc5a41a110b 100644 --- a/contrib/pageinspect/expected/gin.out +++ b/contrib/pageinspect/expected/gin.out @@ -35,6 +35,13 @@ FROM gin_leafpage_items(get_raw_page('test1_y_idx', -[ RECORD 1 ] ?column? | t +-- A posting list that claims to be longer than the page must be reported, +-- rather than read out of bounds. The first list's nbytes is at offset 38. +SELECT gin_leafpage_items(set_byte(set_byte(get_raw_page('test1_y_idx', + (pg_relation_size('test1_y_idx') / + current_setting('block_size')::bigint)::int - 1), + 38, 255), 39, 255)); +ERROR: invalid posting list in GIN data leaf page -- Failure with various modes. -- Suppress the DETAIL message, to allow the tests to work across various -- page sizes and architectures. diff --git a/contrib/pageinspect/expected/gist.out b/contrib/pageinspect/expected/gist.out index de2cf55aae1..11a0d4c992d 100644 --- a/contrib/pageinspect/expected/gist.out +++ b/contrib/pageinspect/expected/gist.out @@ -84,6 +84,16 @@ ERROR: input page is not a valid GiST page -- an invalid (out-of-range, unaligned) line pointer on any architecture. SELECT gist_page_items_bytea(set_byte(set_byte(set_byte(set_byte(get_raw_page('test_gist_idx', 0), 24, 255), 25, 255), 26, 255), 27, 255)); ERROR: invalid line pointer at offset 1 in GiST page +-- A stored value must not reach past the end of its tuple. The index has one +-- tuple, at pd_upper, and its first data byte is the 1-byte varlena header of +-- the range. 0xFF claims 127 bytes on both byte orders. +CREATE TABLE test_gist_range AS SELECT int4range(1, 11) r; +CREATE INDEX test_gist_range_idx ON test_gist_range USING gist (r); +SELECT gist_page_items(set_byte(get_raw_page('test_gist_range_idx', 0), + (SELECT upper FROM page_header(get_raw_page('test_gist_range_idx', 0))) + 8, 255), + 'test_gist_range_idx'); +ERROR: invalid tuple at offset 1 in GiST page +DROP TABLE test_gist_range; \set VERBOSITY default -- Tests with all-zero pages. SHOW block_size \gset diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out index fcf19c5ca5a..ddfbbdc4d53 100644 --- a/contrib/pageinspect/expected/page.out +++ b/contrib/pageinspect/expected/page.out @@ -175,6 +175,12 @@ SELECT * FROM heap_tuple_infomask_flags(x'0010'::int, 0); {HEAP_XMAX_KEYSHR_LOCK} | {} (1 row) +-- A corrupt page header must be rejected, rather than being used to walk off +-- the end of the page. pd_lower is at offset 12. +\set VERBOSITY terse +SELECT heap_page_items(set_byte(set_byte(get_raw_page('test1', 0), 12, 255), 13, 255)); +ERROR: invalid page header +\set VERBOSITY default DROP TABLE test1; -- check that using any of these functions with a partitioned table or index -- would fail diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c index 058ad52b671..5f9b58ea18a 100644 --- a/contrib/pageinspect/ginfuncs.c +++ b/contrib/pageinspect/ginfuncs.c @@ -198,6 +198,7 @@ gin_leafpage_items(PG_FUNCTION_ARGS) mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx); page = get_page_from_raw(raw_page); + verify_page_header(page); if (PageIsNew(page)) { @@ -243,7 +244,7 @@ gin_leafpage_items(PG_FUNCTION_ARGS) fctx = SRF_PERCALL_SETUP(); inter_call_data = fctx->user_fctx; - if (inter_call_data->seg != inter_call_data->lastseg) + if (inter_call_data->seg < inter_call_data->lastseg) { GinPostingList *cur = inter_call_data->seg; HeapTuple resultTuple; @@ -255,6 +256,16 @@ gin_leafpage_items(PG_FUNCTION_ARGS) ItemPointer tids; Datum *tids_datum; + /* + * Check that the segment, and the data it claims to hold, lie within + * the posting lists area. + */ + if ((char *) cur + offsetof(GinPostingList, bytes) > (char *) inter_call_data->lastseg || + (char *) GinNextPostingListSegment(cur) > (char *) inter_call_data->lastseg) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid posting list in GIN data leaf page"))); + memset(nulls, 0, sizeof(nulls)); values[0] = ItemPointerGetDatum(&cur->first); diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c index 975ae24f73a..54094ce9aaa 100644 --- a/contrib/pageinspect/gistfuncs.c +++ b/contrib/pageinspect/gistfuncs.c @@ -36,6 +36,41 @@ PG_FUNCTION_INFO_V1(gist_page_items_bytea); static Page verify_gist_page(bytea *raw_page); +/* + * Check that the null bitmap and stored values of an index tuple lie within + * the tuple, so that index_deform_tuple() does not read past it. + */ +static void +verify_gist_tuple(TupleDesc tupdesc, IndexTuple itup, OffsetNumber offset) +{ + Size len = IndexTupleSize(itup); + Size hoff = IndexInfoFindDataOffset(itup->t_info); + uint8 *bp = (uint8 *) ((char *) itup + sizeof(IndexTupleData)); + Size off = 0; + + if (hoff > len) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("invalid tuple at offset %u in GiST page", offset), + errdetail("Data offset %zu is outside the %zu-byte tuple.", + hoff, len))); + + for (int i = 0; i < tupdesc->natts; i++) + { + Form_pg_attribute att = TupleDescAttr(tupdesc, i); + + if (IndexTupleHasNulls(itup) && att_isnull(i, bp)) + continue; + if (!stored_value_fits((char *) itup + hoff, len - hoff, &off, + att->attlen, att->attalign)) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("invalid tuple at offset %u in GiST page", offset), + errdetail("Stored values extend past the end of the %zu-byte tuple.", + len))); + } +} + /* * Verify that the given bytea contains a GIST page or die in the attempt. * A pointer to the page is returned. @@ -46,6 +81,8 @@ verify_gist_page(bytea *raw_page) Page page = get_page_from_raw(raw_page); GISTPageOpaque opaq; + verify_page_header(page); + if (PageIsNew(page)) return page; @@ -306,6 +343,8 @@ gist_page_items(PG_FUNCTION_ARGS) errmsg("invalid index tuple length at offset %u in GiST page", offset))); + verify_gist_tuple(tupdesc, itup, offset); + index_deform_tuple(itup, tupdesc, itup_values, itup_isnull); diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c index ddd4d8691f1..a6c3d381eee 100644 --- a/contrib/pageinspect/hashfuncs.c +++ b/contrib/pageinspect/hashfuncs.c @@ -61,6 +61,8 @@ verify_hash_page(bytea *raw_page, int flags) Page page = get_page_from_raw(raw_page); int pagetype = LH_UNUSED_PAGE; + verify_page_header(page); + /* Treat new pages as unused. */ if (!PageIsNew(page)) { diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c index 7be4770dc84..4acc670804c 100644 --- a/contrib/pageinspect/heapfuncs.c +++ b/contrib/pageinspect/heapfuncs.c @@ -157,6 +157,7 @@ heap_page_items(PG_FUNCTION_ARGS) inter_call_data->offset = FirstOffsetNumber; inter_call_data->page = get_page_from_raw(raw_page); + verify_page_header(inter_call_data->page); fctx->max_calls = PageGetMaxOffsetNumber(inter_call_data->page); fctx->user_fctx = inter_call_data; diff --git a/contrib/pageinspect/pageinspect.h b/contrib/pageinspect/pageinspect.h index b241fdc97b2..e2934d1c560 100644 --- a/contrib/pageinspect/pageinspect.h +++ b/contrib/pageinspect/pageinspect.h @@ -26,5 +26,8 @@ enum pageinspect_version /* in rawpage.c */ extern Page get_page_from_raw(bytea *raw_page); +extern void verify_page_header(Page page); +extern bool stored_value_fits(const char *data, Size len, Size *off, + int16 typlen, char typalign); #endif /* _PAGEINSPECT_H_ */ diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c index d136593edb2..2de0840a1d4 100644 --- a/contrib/pageinspect/rawpage.c +++ b/contrib/pageinspect/rawpage.c @@ -17,6 +17,7 @@ #include "access/htup_details.h" #include "access/relation.h" +#include "access/tupmacs.h" #include "catalog/namespace.h" #include "catalog/pg_type.h" #include "funcapi.h" @@ -233,6 +234,93 @@ get_page_from_raw(bytea *raw_page) return page; } +/* + * verify_page_header + * + * Check that a page's header is self-consistent. Functions that walk the + * contents of a page must call this first: PageGetMaxOffsetNumber() and the + * other page accessors are all derived from these fields, so a corrupt header + * would make them address memory outside the page. + * + * This is deliberately not done by get_page_from_raw(), so that page_header() + * can still be used to inspect a page whose header is damaged. + */ +void +verify_page_header(Page page) +{ + PageHeader ph = (PageHeader) page; + + /* + * These are the checks PageIsVerified() makes on the header, except for + * the pd_flags test: they are the fields the page accessors derive + * addresses from. An all-zero page passes, and callers handle those. + */ + if (!(ph->pd_lower <= ph->pd_upper && + ph->pd_upper <= ph->pd_special && + ph->pd_special <= BLCKSZ && + ph->pd_special == MAXALIGN(ph->pd_special))) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid page header"), + errdetail("Lower = %u, upper = %u, special = %u.", + ph->pd_lower, ph->pd_upper, ph->pd_special))); +} + +/* + * stored_value_fits + * + * Check that the next stored value of a tuple, of the given type, lies within + * the "len" bytes at "data". "*off" is where the previous value ended, and + * is advanced past this one. This walks the data the same way + * index_deform_tuple() and brin_deform_tuple() do, but checks only lengths, + * not the contents of a value. + */ +bool +stored_value_fits(const char *data, Size len, Size *off, + int16 typlen, char typalign) +{ + Size o = *off; + Size size; + + if (typlen == -1) + { + Size hdrsz; + + if (o >= len) + return false; + o = att_align_pointer(o, typalign, -1, data + o); + if (o >= len || VARATT_IS_EXTERNAL(data + o)) + return false; + hdrsz = VARATT_IS_1B(data + o) ? VARHDRSZ_SHORT : VARHDRSZ; + if (hdrsz > len - o) + return false; + size = VARSIZE_ANY(data + o); + if (size < hdrsz) + return false; + } + else if (typlen == -2) + { + const char *nul; + + if (o >= len) + return false; + nul = memchr(data + o, '\0', len - o); + if (nul == NULL) + return false; + size = nul - (data + o) + 1; + } + else + { + o = att_align_nominal(o, typalign); + size = typlen; + } + + if (o > len || size > len - o) + return false; + *off = o + size; + return true; +} + /* * page_header diff --git a/contrib/pageinspect/sql/brin.sql b/contrib/pageinspect/sql/brin.sql index 4edf17b5a45..03f300e7708 100644 --- a/contrib/pageinspect/sql/brin.sql +++ b/contrib/pageinspect/sql/brin.sql @@ -52,6 +52,22 @@ SELECT (COUNT(*) = (SELECT relpages FROM pg_class WHERE relname = 'test2')) AS r (SELECT (relpages - 1) FROM pg_class WHERE relname = 'test2_a_idx')) AS pages(p), LATERAL brin_page_items(get_raw_page('test2_a_idx', p), 'test2_a_idx') AS items; +-- A corrupt line pointer must be reported, not read out of bounds. All-ones is +-- an invalid (out-of-range, unaligned) line pointer on any architecture. +SELECT brin_page_items(set_byte(set_byte(set_byte(set_byte(get_raw_page('test1_a_idx', 2), 24, 255), 25, 255), 26, 255), 27, 255), 'test1_a_idx'); + +-- A tuple must not reach past the length given by its line pointer. First +-- set the tuple's data offset (the low bits of bt_info) beyond the tuple, then +-- make the first stored value of a text index claim a length beyond the tuple. +-- The tuple is the only one on the page, so it starts at pd_upper. +SELECT brin_page_items(set_byte(get_raw_page('test1_a_idx', 2), + (SELECT upper FROM page_header(get_raw_page('test1_a_idx', 2))) + 4, 31), + 'test1_a_idx'); +CREATE INDEX test1_b_idx ON test1 USING brin (b); +SELECT brin_page_items(set_byte(get_raw_page('test1_b_idx', 2), + (SELECT upper FROM page_header(get_raw_page('test1_b_idx', 2))) + 8, 255), + 'test1_b_idx'); + DROP TABLE test1; DROP TABLE test2; diff --git a/contrib/pageinspect/sql/gin.sql b/contrib/pageinspect/sql/gin.sql index b57466d7ebf..efd923624d6 100644 --- a/contrib/pageinspect/sql/gin.sql +++ b/contrib/pageinspect/sql/gin.sql @@ -18,6 +18,13 @@ FROM gin_leafpage_items(get_raw_page('test1_y_idx', (pg_relation_size('test1_y_idx') / current_setting('block_size')::bigint)::int - 1)); +-- A posting list that claims to be longer than the page must be reported, +-- rather than read out of bounds. The first list's nbytes is at offset 38. +SELECT gin_leafpage_items(set_byte(set_byte(get_raw_page('test1_y_idx', + (pg_relation_size('test1_y_idx') / + current_setting('block_size')::bigint)::int - 1), + 38, 255), 39, 255)); + -- Failure with various modes. -- Suppress the DETAIL message, to allow the tests to work across various -- page sizes and architectures. diff --git a/contrib/pageinspect/sql/gist.sql b/contrib/pageinspect/sql/gist.sql index 9dd79ff9450..b43c426ea55 100644 --- a/contrib/pageinspect/sql/gist.sql +++ b/contrib/pageinspect/sql/gist.sql @@ -37,6 +37,15 @@ SELECT gist_page_items_bytea(get_raw_page('test_gist_btree', 0)); -- A corrupt line pointer must be reported, not read out of bounds. All-ones is -- an invalid (out-of-range, unaligned) line pointer on any architecture. SELECT gist_page_items_bytea(set_byte(set_byte(set_byte(set_byte(get_raw_page('test_gist_idx', 0), 24, 255), 25, 255), 26, 255), 27, 255)); +-- A stored value must not reach past the end of its tuple. The index has one +-- tuple, at pd_upper, and its first data byte is the 1-byte varlena header of +-- the range. 0xFF claims 127 bytes on both byte orders. +CREATE TABLE test_gist_range AS SELECT int4range(1, 11) r; +CREATE INDEX test_gist_range_idx ON test_gist_range USING gist (r); +SELECT gist_page_items(set_byte(get_raw_page('test_gist_range_idx', 0), + (SELECT upper FROM page_header(get_raw_page('test_gist_range_idx', 0))) + 8, 255), + 'test_gist_range_idx'); +DROP TABLE test_gist_range; \set VERBOSITY default -- Tests with all-zero pages. diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql index c75fe1147f6..b34c9a13f36 100644 --- a/contrib/pageinspect/sql/page.sql +++ b/contrib/pageinspect/sql/page.sql @@ -62,6 +62,12 @@ SELECT * FROM heap_tuple_infomask_flags(0, 0); -- no combined flags SELECT * FROM heap_tuple_infomask_flags(x'0010'::int, 0); +-- A corrupt page header must be rejected, rather than being used to walk off +-- the end of the page. pd_lower is at offset 12. +\set VERBOSITY terse +SELECT heap_page_items(set_byte(set_byte(get_raw_page('test1', 0), 12, 255), 13, 255)); +\set VERBOSITY default + DROP TABLE test1; -- check that using any of these functions with a partitioned table or index -- 2.37.1 (Apple Git-137.1)