agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedFrom: PG Bug reporting form <noreply@postgresql.org>
To: pgsql-bugs@lists.postgresql.org
Cc: malis@pgrust.com
Subject: BUG #19599: RestoreBlockImage: the decode cross-checks never bound hole_offset + hole_length against BLCKSZ
Date: Sun, 02 Aug 2026 17:51:51 +0000
Message-ID: <19599-8859c3822a831331@postgresql.org> (raw)
The following bug has been logged on the website:
Bug reference: 19599
Logged by: Michael Malis
Email address: malis@pgrust.com
PostgreSQL version: 18.3
Operating system: Debian
Description:
I don't think this is a real issue because it requires a specific WAL
structure, but I figured I would report it anyway.
DecodeXLogRecord cross-checks a block image's hole descriptor for
non-zero-ness only. It never checks that the hole fits inside the page. A
record that sets both hole_offset and hole_length large — each is a uint16,
so up to 65535 — passes validation, and RestoreBlockImage then uses those
values directly as memcpy/MemSet offsets and lengths into an 8 kB page
buffer.
Evidence status — please read
-----------------------------
- What we verified: the missing bound, by reading 18.3. Line numbers below.
- What we did NOT do: execute the overrun. Our differential harness detects
the out-of-bounds geometry and skips the C oracle for those inputs
precisely so it never drives undefined behaviour, so no ASan/valgrind
report exists and we cannot attach one. We also have no live-server
reproducer, because constructing the record requires authoring WAL with a
valid CRC over the malformed body — our builder does this inside the
harness, not against a server.
- What is positively demonstrated: our reimplementation rejects this
geometry, and that rejection is asserted under fuzzing (one-sided). That
is evidence the input class is reachable through decode, not evidence
about C's behaviour.
We would rather file this as "here is a missing check, here is the input
that reaches it" than overstate it. If you want the overrun demonstrated
under a sanitizer before considering it, that is a reasonable ask and we can
do it.
Reproducer (harness-level)
--------------------------
A WAL record whose block-image header carries, with a valid CRC over the
body:
bimg_len = 16
hole_offset = 8000
hole_length = 8000 (8000 + 8000 = 16000 > BLCKSZ 8192)
bimg_info = BKPIMAGE_HAS_HOLE | BKPIMAGE_APPLY |
BKPIMAGE_COMPRESS_PGLZ (0x07)
hole_length is an on-wire field only for COMPRESSED + HAS_HOLE images, which
is why the shape is specifically a compressed image.
Expected vs. actual
-------------------
- Expected: decode rejects the record with an invalid-state error, as it
does for the zero-valued cases it already checks.
- Actual (by inspection): decode accepts it and the reconstruction
arithmetic runs with a hole larger than the page.
Mechanism, with file:line into the 18.3 source
----------------------------------------------
Field widths, src/include/access/xlogreader.h:
139: uint16 hole_offset;
140: uint16 hole_length;
141: uint16 bimg_len;
The cross-checks, xlogreader.c — the comment states exactly what is checked:
1826: /*
1827: * cross-check that hole_offset > 0, hole_length > 0 and
1828: * bimg_len < BLCKSZ if the HAS_HOLE flag is set.
1829: */
1830: if ((blk->bimg_info & BKPIMAGE_HAS_HOLE) &&
1831: (blk->hole_offset == 0 ||
1832: blk->hole_length == 0 ||
1833: blk->bimg_len == BLCKSZ))
Three non-zero-ness conditions; no relation between the two fields and
BLCKSZ.
The consumer, RestoreBlockImage:
2170: memcpy(page, ptr, bkpb->hole_offset);
2172: MemSet(page + bkpb->hole_offset, 0, bkpb->hole_length);
2173: memcpy(page + (bkpb->hole_offset + bkpb->hole_length),
2174: ptr + bkpb->hole_offset,
2175: BLCKSZ - (bkpb->hole_offset + bkpb->hole_length));
page is BLCKSZ. With hole_offset = 8000 the first memcpy alone exceeds the
page. The third length, BLCKSZ - (hole_offset + hole_length), is negative
and converts to a very large size_t.
A second path through the same missing bound: the decompressors are handed
BLCKSZ - bkpb->hole_length as their output capacity into a BLCKSZ-sized tmp:
2109: if (pglz_decompress(ptr, bkpb->bimg_len, tmp.data,
2110: BLCKSZ - bkpb->hole_length, true) <
0)
2117: LZ4_decompress_safe(ptr, tmp.data, bkpb->bimg_len,
BLCKSZ - bkpb->hole_length)
2131: ZSTD_decompress(tmp.data, BLCKSZ - bkpb->hole_length,
ptr, bkpb->bimg_len)
With hole_length > BLCKSZ that expression underflows, so a decompressor is
told it has far more room than it does.
view thread (3+ messages) latest in thread
Message-ID: <19599-8859c3822a831331@postgresql.org>
Permalink: ../19599-8859c3822a831331@postgresql.org/
Also on: postgresql.org/message-id/19599-8859c3822a831331@postgresql.org
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: pgsql-bugs@postgresql.org
Cc: noreply@postgresql.org, pgsql-bugs@lists.postgresql.org, malis@pgrust.com
Subject: Re: BUG #19599: RestoreBlockImage: the decode cross-checks never bound hole_offset + hole_length against BLCKSZ
In-Reply-To: <19599-8859c3822a831331@postgresql.org>
* 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