Hi hackers, ginPostingListDecodeAllSegments() sets endptr from segment->nbytes and steps to the next segment with GinNextPostingListSegment(). Neither checked against the end of the posting list, and decode_varbyte() has no end pointer. So a corrupt page is read past its end, and its items are decoded without being checked.
A corrupt page reaches this code through an index scan, VACUUM or WAL replay. It is also directly reachable through pageinspect's gin_leafpage_items(), which makes it convenient to reproduce with crafted bytes as in the tests. On a release build it returns invalid TIDs with no error. On an assert build it aborts. I reproduced on 14 through 18 release builds and on master. I don't think this is a security issue. The normal paths require an already-corrupt posting list, and the direct crafted-input path is through pageinspect. Seven patches: 0001 bound decode_varbyte() against the segment end 0002 check each segment fits within the posting list 0003 reject items that are out of range or out of order 0004 tests, through gin_leafpage_items() 0005 size the output array from len, not the unvalidated nbytes 0006 remove unreachable repalloc_array() 0007 scope some locals to the loop (cosmetic) I split these deliberately as it made it easier for me to see what each one fixes and understand the code along the way. Some may be worth combining. The change to decode_varbyte() replaces the seven nested if-blocks with a loop. I tried adding the endptr checks in each block with a shared goto label for the ereport(), but it was kind of unwieldy to read through. There was no measurable difference between the two approaches either. 0001 through 0003 also change what happens if a corrupt posting list is encountered during WAL replay. The corruption now raises an error and stops recovery instead of continuing after decoding garbage. That seems preferable, but it is a behavioral change. Sizing the result array from len (0005) should not over-allocate in practice. A dense posting list runs about a byte per item, so len is within a few percent of the item count, and the array is a transient allocation bounded by the page anyway. For backpatching, 0007 is cosmetic and master only. 0001 through 0005 are the fix and its test, all reachable back to 14, though the replay behavior above is worth weighing before backpatching them. Regards, -- Sehrope Sarkuni Founder & CEO | JackDB, Inc. | https://www.jackdb.com/
From d574ce3d77d97535cd45f73cd9ab2d41c037a066 Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sat, 26 Sep 2026 18:56:26 +0000 Subject: [PATCH v1 5/7] gin: size the posting list decode output array from len ginPostingListDecodeAllSegments() sized its output array from the first segment's nbytes, read before the loop had checked that the segment lies within the posting list, so a len smaller than a segment header read nbytes out of bounds. len bounds the item count, since every item takes at least one byte, so size the array from len instead. This also avoids repalloc() on a valid multi-segment list. --- src/backend/access/gin/ginpostinglist.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/backend/access/gin/ginpostinglist.c b/src/backend/access/gin/ginpostinglist.c index e66f69d04f7..56bb8c1e6eb 100644 --- a/src/backend/access/gin/ginpostinglist.c +++ b/src/backend/access/gin/ginpostinglist.c @@ -279,9 +279,12 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ unsigned char *endptr; /* - * Guess an initial size of the array. + * Size from len, not segment->nbytes. len can be smaller than a segment + * header, so reading nbytes here could run off the buffer. len also + * bounds the item count, since every item costs at least a byte, so a + * valid list never grows the array. */ - nallocated = segment->nbytes * 2 + 1; + nallocated = Max(len, 1); result = palloc_array(ItemPointerData, nallocated); ndecoded = 0; -- 2.17.1
From 8d9fc7376ee8a10f949e62c2e0b9199e3dff48e3 Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sat, 26 Sep 2026 19:23:14 +0000 Subject: [PATCH v1 3/7] gin: reject invalid items in a decoded posting list A corrupt page can hold a posting list that stays within bounds but decodes to invalid items. Such as a first item or an accumulated value that yields offset 0, or an item that does not exceed its predecessor. These were only Assert()ed, so they aborted an assert build and returned garbage otherwise. Reject them with an error instead. --- src/backend/access/gin/ginpostinglist.c | 35 +++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/backend/access/gin/ginpostinglist.c b/src/backend/access/gin/ginpostinglist.c index f7dec6318c9..e66f69d04f7 100644 --- a/src/backend/access/gin/ginpostinglist.c +++ b/src/backend/access/gin/ginpostinglist.c @@ -287,6 +287,9 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ ndecoded = 0; while ((char *) segment < endseg) { + OffsetNumber firstoff; + uint64 prev; + /* * Reject a segment that runs past the end of the posting list. * Compare sizes rather than forming segment + @@ -300,6 +303,23 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ (errcode(ERRCODE_DATA_CORRUPTED), errmsg("corrupted GIN posting list"))); + /* + * The first item's offset must fit in MaxHeapTuplesPerPageBits, as + * itemptr_to_uint64() below requires. That is tighter than + * OffsetNumberIsValid(), which is why the range is open-coded here. + * Read it with the No-Check accessor, since the checking one would + * Assert() on the corrupt value being rejected. The last clause keeps + * items ascending across segment boundaries. + */ + firstoff = GinItemPointerGetOffsetNumber(&segment->first); + if (firstoff == InvalidOffsetNumber || + firstoff >= (1 << MaxHeapTuplesPerPageBits) || + (ndecoded > 0 && + ginCompareItemPointers(&segment->first, &result[ndecoded - 1]) <= 0)) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("corrupted GIN posting list"))); + /* enlarge output array if needed */ if (ndecoded >= nallocated) { @@ -308,12 +328,11 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ } /* copy the first item */ - Assert(OffsetNumberIsValid(ItemPointerGetOffsetNumber(&segment->first))); - Assert(ndecoded == 0 || ginCompareItemPointers(&segment->first, &result[ndecoded - 1]) > 0); result[ndecoded] = segment->first; ndecoded++; val = itemptr_to_uint64(&segment->first); + prev = val; ptr = segment->bytes; endptr = segment->bytes + segment->nbytes; while (ptr < endptr) @@ -327,6 +346,18 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ val += decode_varbyte(&ptr, endptr); + /* + * Reject offset 0 and a non-increasing item. Neither appears in a + * valid list, and uint64_to_itemptr() below Asserts on offset 0, so + * this has to run before it. + */ + if ((val & ((1 << MaxHeapTuplesPerPageBits) - 1)) == 0 || + val <= prev) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("corrupted GIN posting list"))); + prev = val; + uint64_to_itemptr(val, &result[ndecoded]); ndecoded++; } -- 2.17.1
From 82942e629508a2f78b19e960e2d8ffaa55152cb1 Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sun, 27 Sep 2026 12:48:04 +0000 Subject: [PATCH v1 2/7] gin: check each posting list segment fits before decoding it ginPostingListDecodeAllSegments() set endptr from segment->nbytes and stepped to the next segment with GinNextPostingListSegment() without checking either against the end of the posting list, so a corrupt page could be read past its end. Check that each segment header and the nbytes it claims lie within the remaining length first, comparing sizes so no out-of-bounds pointer is formed. --- src/backend/access/gin/ginpostinglist.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/backend/access/gin/ginpostinglist.c b/src/backend/access/gin/ginpostinglist.c index 89da084bbe8..f7dec6318c9 100644 --- a/src/backend/access/gin/ginpostinglist.c +++ b/src/backend/access/gin/ginpostinglist.c @@ -287,6 +287,19 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ ndecoded = 0; while ((char *) segment < endseg) { + /* + * Reject a segment that runs past the end of the posting list. + * Compare sizes rather than forming segment + + * SizeOfGinPostingList(segment), which would be an out-of-bounds + * pointer. The header is tested before nbytes is read, and the loop + * condition keeps endseg - segment positive for the unsigned cast. + */ + if (offsetof(GinPostingList, bytes) > (Size) (endseg - (char *) segment) || + SizeOfGinPostingList(segment) > (Size) (endseg - (char *) segment)) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("corrupted GIN posting list"))); + /* enlarge output array if needed */ if (ndecoded >= nallocated) { -- 2.17.1
From 5d7eb3cc2e0fb36bc731dfe08b68068389cef4d5 Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sun, 27 Sep 2026 12:48:04 +0000 Subject: [PATCH v1 1/7] gin: bound decode_varbyte() against the segment end decode_varbyte() had no end pointer and stopped only on a byte without the continuation bit set, so a corrupt stream ran past the segment. Pass the segment end in and stop at it, treating a truncated integer or one longer than the encoding can produce as corruption. The seven-way nesting becomes a loop. --- src/backend/access/gin/ginpostinglist.c | 56 +++++++------------------ 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/src/backend/access/gin/ginpostinglist.c b/src/backend/access/gin/ginpostinglist.c index 07656574029..89da084bbe8 100644 --- a/src/backend/access/gin/ginpostinglist.c +++ b/src/backend/access/gin/ginpostinglist.c @@ -130,51 +130,25 @@ encode_varbyte(uint64 val, unsigned char **ptr) * Decode varbyte-encoded integer at *ptr. *ptr is incremented to next integer. */ static uint64 -decode_varbyte(unsigned char **ptr) +decode_varbyte(unsigned char **ptr, unsigned char *endptr) { - uint64 val; + uint64 val = 0; unsigned char *p = *ptr; - uint64 c; - /* 1st byte */ - c = *(p++); - val = c & 0x7F; - if (c & 0x80) + for (int i = 0;; i++) { - /* 2nd byte */ + uint64 c; + + if (p >= endptr || i >= MaxBytesPerInteger) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("corrupted GIN posting list"))); + c = *(p++); - val |= (c & 0x7F) << 7; - if (c & 0x80) - { - /* 3rd byte */ - c = *(p++); - val |= (c & 0x7F) << 14; - if (c & 0x80) - { - /* 4th byte */ - c = *(p++); - val |= (c & 0x7F) << 21; - if (c & 0x80) - { - /* 5th byte */ - c = *(p++); - val |= (c & 0x7F) << 28; - if (c & 0x80) - { - /* 6th byte */ - c = *(p++); - val |= (c & 0x7F) << 35; - if (c & 0x80) - { - /* 7th byte, should not have continuation bit */ - c = *(p++); - val |= c << 42; - Assert((c & 0x80) == 0); - } - } - } - } - } + val |= (c & 0x7F) << (7 * i); + + if ((c & 0x80) == 0) + break; } *ptr = p; @@ -338,7 +312,7 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ result = repalloc_array(result, ItemPointerData, nallocated); } - val += decode_varbyte(&ptr); + val += decode_varbyte(&ptr, endptr); uint64_to_itemptr(val, &result[ndecoded]); ndecoded++; -- 2.17.1
From 6be1bf5c3d245dcd2f01f345c716ddf843cc0d13 Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Fri, 25 Sep 2026 17:29:58 +0000 Subject: [PATCH v1 4/7] pageinspect: add tests for corrupt GIN posting lists Check that gin_leafpage_items() reports a corrupted GIN posting list rather than aborting or decoding past the page, for an unterminated varbyte stream, an item that decodes to offset 0, and a first item with offset 0. --- contrib/pageinspect/expected/gin.out | 22 ++++++++++++++++++++++ contrib/pageinspect/sql/gin.sql | 19 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/contrib/pageinspect/expected/gin.out b/contrib/pageinspect/expected/gin.out index ff1da6a5a17..c1582cbea36 100644 --- a/contrib/pageinspect/expected/gin.out +++ b/contrib/pageinspect/expected/gin.out @@ -53,6 +53,28 @@ SELECT * FROM gin_page_opaque_info(get_raw_page('test1', 0)); ERROR: input page is not a valid GIN data leaf page SELECT * FROM gin_leafpage_items(get_raw_page('test1', 0)); ERROR: input page is not a valid GIN data leaf page +-- corrupt posting list on the last leaf page. The first GinPostingList +-- begins 32 bytes into the page, so on any block size its first item's +-- offset is at bytes 36-37 and its varbyte stream starts at byte 40 +-- (set_byte is 0-based, overlay 1-based). Each of these must be reported, +-- not decoded past. +SELECT (pg_relation_size('test1_y_idx') / + current_setting('block_size')::bigint)::int - 1 AS ln \gset +-- a varbyte stream that never terminates (seven continuation bytes) +SELECT gin_leafpage_items(overlay(get_raw_page('test1_y_idx', :ln) + PLACING '\x80808080808080'::bytea FROM 41)); +ERROR: corrupted GIN posting list +-- an item that decodes to offset 0, with the first item's offset set to 1 +-- and the first delta to 2047, so the running value reaches 2048 +SELECT gin_leafpage_items(set_byte(set_byte(set_byte(set_byte( + get_raw_page('test1_y_idx', :ln), + 36, 1), 37, 0), 40, 255), 41, 15)); +ERROR: corrupted GIN posting list +-- a segment whose first item has an invalid offset of 0 +SELECT gin_leafpage_items(set_byte(set_byte( + get_raw_page('test1_y_idx', :ln), + 36, 0), 37, 0)); +ERROR: corrupted GIN posting list \set VERBOSITY default -- Tests with all-zero pages. SHOW block_size \gset diff --git a/contrib/pageinspect/sql/gin.sql b/contrib/pageinspect/sql/gin.sql index b57466d7ebf..5f41ba54b34 100644 --- a/contrib/pageinspect/sql/gin.sql +++ b/contrib/pageinspect/sql/gin.sql @@ -30,6 +30,25 @@ SELECT gin_page_opaque_info('ccc'::bytea); SELECT * FROM gin_metapage_info(get_raw_page('test1', 0)); SELECT * FROM gin_page_opaque_info(get_raw_page('test1', 0)); SELECT * FROM gin_leafpage_items(get_raw_page('test1', 0)); +-- corrupt posting list on the last leaf page. The first GinPostingList +-- begins 32 bytes into the page, so on any block size its first item's +-- offset is at bytes 36-37 and its varbyte stream starts at byte 40 +-- (set_byte is 0-based, overlay 1-based). Each of these must be reported, +-- not decoded past. +SELECT (pg_relation_size('test1_y_idx') / + current_setting('block_size')::bigint)::int - 1 AS ln \gset +-- a varbyte stream that never terminates (seven continuation bytes) +SELECT gin_leafpage_items(overlay(get_raw_page('test1_y_idx', :ln) + PLACING '\x80808080808080'::bytea FROM 41)); +-- an item that decodes to offset 0, with the first item's offset set to 1 +-- and the first delta to 2047, so the running value reaches 2048 +SELECT gin_leafpage_items(set_byte(set_byte(set_byte(set_byte( + get_raw_page('test1_y_idx', :ln), + 36, 1), 37, 0), 40, 255), 41, 15)); +-- a segment whose first item has an invalid offset of 0 +SELECT gin_leafpage_items(set_byte(set_byte( + get_raw_page('test1_y_idx', :ln), + 36, 0), 37, 0)); \set VERBOSITY default -- Tests with all-zero pages. -- 2.17.1
From 2dab9fee061dd0cedbfc44caab8d31fb485c4f7a Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sat, 26 Sep 2026 19:15:05 +0000 Subject: [PATCH v1 7/7] gin: scope decode locals to the segment loop val, ptr and endptr live only within one iteration of the segment loop, like prev. Declare them there rather than at the top of the function. --- src/backend/access/gin/ginpostinglist.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/backend/access/gin/ginpostinglist.c b/src/backend/access/gin/ginpostinglist.c index 57c16eb05cb..4aeeb8c4eef 100644 --- a/src/backend/access/gin/ginpostinglist.c +++ b/src/backend/access/gin/ginpostinglist.c @@ -272,11 +272,8 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ { ItemPointer result; int nallocated; - uint64 val; char *endseg = ((char *) segment) + len; int ndecoded; - unsigned char *ptr; - unsigned char *endptr; /* * Size from len, not segment->nbytes. len can be smaller than a segment @@ -291,7 +288,10 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ while ((char *) segment < endseg) { OffsetNumber firstoff; + uint64 val; uint64 prev; + unsigned char *ptr; + unsigned char *endptr; /* * Reject a segment that runs past the end of the posting list. -- 2.17.1
From 5d92eada65fdca559ca7b6d50f84f9eed226b048 Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sun, 27 Sep 2026 12:39:29 -0400 Subject: [PATCH v1 6/7] gin: replace unreachable repalloc_array() with Assert() Now that we size our result array based on the len of all segments, we should never need to expand the result array to accomodate more elements. So we replace that with an Assert() that the array has enough capacity for another element. --- src/backend/access/gin/ginpostinglist.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/backend/access/gin/ginpostinglist.c b/src/backend/access/gin/ginpostinglist.c index 56bb8c1e6eb..57c16eb05cb 100644 --- a/src/backend/access/gin/ginpostinglist.c +++ b/src/backend/access/gin/ginpostinglist.c @@ -323,14 +323,8 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ (errcode(ERRCODE_DATA_CORRUPTED), errmsg("corrupted GIN posting list"))); - /* enlarge output array if needed */ - if (ndecoded >= nallocated) - { - nallocated *= 2; - result = repalloc_array(result, ItemPointerData, nallocated); - } - /* copy the first item */ + Assert(ndecoded < nallocated); result[ndecoded] = segment->first; ndecoded++; @@ -340,13 +334,6 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ endptr = segment->bytes + segment->nbytes; while (ptr < endptr) { - /* enlarge output array if needed */ - if (ndecoded >= nallocated) - { - nallocated *= 2; - result = repalloc_array(result, ItemPointerData, nallocated); - } - val += decode_varbyte(&ptr, endptr); /* @@ -361,6 +348,7 @@ ginPostingListDecodeAllSegments(GinPostingList *segment, int len, int *ndecoded_ errmsg("corrupted GIN posting list"))); prev = val; + Assert(ndecoded < nallocated); uint64_to_itemptr(val, &result[ndecoded]); ndecoded++; } -- 2.17.1
