public inbox for [email protected]
help / color / mirror / Atom feedFrom: surya poondla <[email protected]>
To: [email protected] <[email protected]>
Cc: [email protected]
Subject: Re: Fw: Re: heap_force_common in contrib/pg_surgery/heap_surgery.c has an off by one stack buffer overflow
Date: Wed, 3 Jun 2026 15:31:27 -0700
Message-ID: <CAOVWO5p-nQ2ki88uAUO5TNWNZDmX-ZZZmJ3307K0xnsg4q75rA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
Hi 王跃林,
Thank you for reporting the issue, I am able to reproduce it on master.
The include_this_tid[] array is sized MaxHeapTuplesPerPage but indexed
using 1-based OffsetNumber,
so the largest legal offset (MaxHeapTuplesPerPage itself) lands one slot
past the end.
psql (19beta1)
Type "help" for help.
postgres=# CREATE EXTENSION IF NOT EXISTS pg_surgery;
CREATE EXTENSION
postgres=# CREATE TABLE vuln_005_t();
CREATE TABLE
postgres=# INSERT INTO vuln_005_t SELECT FROM generate_series(1, 291);
INSERT 0 291
postgres=# SELECT heap_force_freeze('vuln_005_t'::regclass, ARRAY['(0,
291)']::tid[]);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.
!?> q
-?> q
-?>
!?> quit
Proposed patch attached. It does two things:
1. Resize include_this_tid[] to MaxHeapTuplesPerPage + 1 so every legal
1-based offset has a slot. This removes the structural off-by-one
2. Extend the per-TID input check to also reject offno > MaxHeapTuplesPerPage,
so a corrupted page whose pd_lower lets max offset exceed the structural
maximum cannot reach the array either.
With the patch I no longer see the crash
postgres=# DROP TABLE IF EXISTS vuln_005_t;
DROP TABLE
postgres=# DROP EXTENSION IF EXISTS pg_surgery;
DROP EXTENSION
postgres=#
postgres=# CREATE EXTENSION pg_surgery;
CREATE EXTENSION
postgres=# CREATE TABLE vuln_005_t();
CREATE TABLE
postgres=# INSERT INTO vuln_005_t SELECT FROM generate_series(1, 291);
INSERT 0 291
postgres=# SELECT count(*) FROM vuln_005_t;
count
-------
291
(1 row)
postgres=# SELECT heap_force_freeze('vuln_005_t'::regclass, ARRAY['(0,
291)']::tid[]);
heap_force_freeze
-------------------
(1 row)
Regards,
Surya Poondla
Attachments:
[application/octet-stream] 0001-Fix-off-by-one-stack-buffer-overflow-in-pg_surgery.patch (2.2K, 3-0001-Fix-off-by-one-stack-buffer-overflow-in-pg_surgery.patch)
download | inline diff:
From b4c62d4d32683341b3a344936e8ac8ad958d9f7b Mon Sep 17 00:00:00 2001
From: spoondla <[email protected]>
Date: Wed, 3 Jun 2026 15:17:00 -0700
Subject: [PATCH] Fix off-by-one stack buffer overflow in pg_surgery Issue:
heap_force_common() declared include_this_tid[] with MaxHeapTuplesPerPage
slots and indexed it directly with a 1-based OffsetNumber, so an input TID
whose offset number equals MaxHeapTuplesPerPage wrote one byte past the end
of the stack array and this woulod cause a seg fault and crash postgres
Fix:
Updated the size of include_this_tid[] as MaxHeapTuplesPerPage + 1 so every legal
1-based offset has a slot, and reject offno > MaxHeapTuplesPerPage in
the per-TID validation so a corrupted page header that reports an
impossibly large maxoffset cannot reach the array either.
---
contrib/pg_surgery/heap_surgery.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/contrib/pg_surgery/heap_surgery.c b/contrib/pg_surgery/heap_surgery.c
index ae4e7c0136c..19169131815 100644
--- a/contrib/pg_surgery/heap_surgery.c
+++ b/contrib/pg_surgery/heap_surgery.c
@@ -92,7 +92,8 @@ heap_force_common(FunctionCallInfo fcinfo, HeapTupleForceOption heap_force_opt)
Relation rel;
OffsetNumber curr_start_ptr,
next_start_ptr;
- bool include_this_tid[MaxHeapTuplesPerPage];
+ /* Sized +1 because OffsetNumbers are 1-based and can reach MaxHeapTuplesPerPage. */
+ bool include_this_tid[MaxHeapTuplesPerPage + 1];
if (RecoveryInProgress())
ereport(ERROR,
@@ -193,7 +194,9 @@ heap_force_common(FunctionCallInfo fcinfo, HeapTupleForceOption heap_force_opt)
ItemId itemid;
/* Check whether the offset number is valid. */
- if (offno == InvalidOffsetNumber || offno > maxoffset)
+ if (offno == InvalidOffsetNumber ||
+ offno > maxoffset ||
+ offno > MaxHeapTuplesPerPage)
{
ereport(NOTICE,
errmsg("skipping tid (%u, %u) for relation \"%s\" because the item number is out of range",
@@ -228,7 +231,7 @@ heap_force_common(FunctionCallInfo fcinfo, HeapTupleForceOption heap_force_opt)
}
/* Mark it for processing. */
- Assert(offno < MaxHeapTuplesPerPage);
+ Assert(offno <= MaxHeapTuplesPerPage);
include_this_tid[offno] = true;
}
--
2.39.5 (Apple Git-154)
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Fw: Re: heap_force_common in contrib/pg_surgery/heap_surgery.c has an off by one stack buffer overflow
In-Reply-To: <CAOVWO5p-nQ2ki88uAUO5TNWNZDmX-ZZZmJ3307K0xnsg4q75rA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox