Re: Fw: Re: heap_force_common in contrib/pg_surgery/heap_surgery.c has an off by one stack buffer overflow - Mailing list pgsql-bugs

From 王跃林
Subject Re: Fw: Re: heap_force_common in contrib/pg_surgery/heap_surgery.c has an off by one stack buffer overflow
Date
Msg-id ACIALACwKVHP*kijcia20KrV.3.1780541225292.Hmail.3020001251@tju.edu.cn
Whole thread
In response to Re: Fw: Re: heap_force_common in contrib/pg_surgery/heap_surgery.c has an off by one stack buffer overflow  (surya poondla <suryapoondla4@gmail.com>)
List pgsql-bugs

Thanks for your reply!





王跃林
3020001251@tju.edu.cn



Original:
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

pgsql-bugs by date:

Previous
From: surya poondla
Date:
Subject: Re: Fw: Re: heap_force_common in contrib/pg_surgery/heap_surgery.c has an off by one stack buffer overflow
Next
From: Antonin Houska
Date:
Subject: Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API