From: Grigorev Jurij <ju.grigorev@ftdata.ru>
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Cc: Michael Malis <malis@pgrust.com>
Subject: Re: BUG #19599: RestoreBlockImage: the decode cross-checks never bound hole_offset + hole_length against BLCKSZ
Date: Fri, 4 Sep 2026 08:01:05 +0000
Message-ID: <cc0a5886d2a74f99ae58c1647ea5ed8b@localhost.localdomain> (raw)
Hi,
Michael reported this as BUG #19599. He wasn't sure it was a real issue, but
I think the missing check is still worth fixing. Postgres never writes a
hole that doesn't fit in the page, but DecodeXLogRecord() will happily
accept one from a corrupt or hand-built record, and RestoreBlockImage()
then uses those fields as memcpy/MemSet lengths.
What decode checks today is only non-zero values:
> if ((blk->bimg_info & BKPIMAGE_HAS_HOLE) &&
> (blk->hole_offset == 0 ||
> blk->hole_length == 0 ||
> blk->bimg_len == BLCKSZ))
It never asks whether the hole actually fits in the page. Both fields
are uint16s taken from the record, so this gets through:
hole_offset = 8000
hole_length = 8000 /* 16000 > BLCKSZ */
bimg_len = 16
bimg_info = HAS_HOLE | APPLY | COMPRESS_PGLZ
That's specifically a compressed image, because that's the only shape
where hole_length is stored in the WAL rather than computed. The CRC
can still be valid. After that, RestoreBlockImage() does:
memcpy(page, ptr, hole_offset);
MemSet(page + hole_offset, 0, hole_length);
memcpy(..., BLCKSZ - (hole_offset + hole_length));
With the numbers above, the first memcpy already runs off the end of an
8kB page, and the last length underflows to a huge size_t. The
decompressors have the same problem: they are told the output buffer is
BLCKSZ - hole_length bytes.
I've attached the patch that adds the missing bound to that existing HAS_HOLE
check:
hole_offset > BLCKSZ ||
hole_length > BLCKSZ - hole_offset
(subtraction rather than addition, so the two uint16s can't overflow.)
The same check is repeated at the start of RestoreBlockImage(), before
decompression or memcpy. A bad hole still uses the existing HAS_HOLE
error message, I didn't add a new one.
I also have a small frontend test that builds this record in memory (valid
header and CRC, compressed image, bad hole) and feeds it to
DecodeXLogRecord(). Happy to send that if it's useful, I left it out of
this mail so the patch stays small.
Does this look like the right approach?
I'd also like to hear whether it's worth back-patching.
Thanks,
Yuriy Grigoryev=
Attachments:0001-Reject-WAL-block-images-whose-hole-does-not-fit.patch (application/octet-stream, 3.3K) download
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Yuriy Grigoryev <ju.grigorev@ftdata.ru>
Date: Fri, 4 Sep 2026 14:20:00 +0700
Subject: [PATCH] Reject WAL block images whose hole does not fit in the page.
DecodeXLogRecord() already rejects a BKPIMAGE_HAS_HOLE image when
hole_offset or hole_length is zero, or when bimg_len is BLCKSZ. It
does not check that the hole lies inside the page. hole_offset and
hole_length are uint16 values taken from the WAL record, so a
compressed image can claim a hole that starts near the end of the
page and extends well past BLCKSZ.
RestoreBlockImage() then uses those fields as memcpy/MemSet lengths
into a BLCKSZ buffer, and as BLCKSZ - hole_length for the
decompressor's output capacity. A hole that does not fit causes
out-of-bounds writes and size_t underflow.
Extend the existing HAS_HOLE cross-check with hole_offset <= BLCKSZ
and hole_length <= BLCKSZ - hole_offset, using subtraction so the
two untrusted fields are never added together. Re-check the same
bound at the start of RestoreBlockImage() before decompression or
memcpy.
This is not reachable from WAL produced by a healthy PostgreSQL
instance. It requires a corrupt or crafted record that still has a
plausible structure.
Discussion: https://postgr.es/m/19599-8859c3822a831331@postgresql.org
---
src/backend/access/transam/xlogreader.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.cindex 5c26d33a603..b7d082d8f8b 100644--- a/src/backend/access/transam/xlogreader.c+++ b/src/backend/access/transam/xlogreader.c@@ -1824,13 +1824,18 @@ DecodeXLogRecord(XLogReaderState *state,
datatotal += blk->bimg_len;
/*
- * cross-check that hole_offset > 0, hole_length > 0 and- * bimg_len < BLCKSZ if the HAS_HOLE flag is set.+ * cross-check that hole_offset > 0, hole_length > 0,+ * bimg_len < BLCKSZ, and the hole fits in the page if the+ * HAS_HOLE flag is set. Compare hole_length with+ * BLCKSZ - hole_offset so the two untrusted fields are never+ * added together.
*/
if ((blk->bimg_info & BKPIMAGE_HAS_HOLE) &&
(blk->hole_offset == 0 ||
blk->hole_length == 0 ||
- blk->bimg_len == BLCKSZ))+ blk->bimg_len == BLCKSZ ||+ blk->hole_offset > BLCKSZ ||+ blk->hole_length > BLCKSZ - blk->hole_offset))
{
report_invalid_record(state,
"BKPIMAGE_HAS_HOLE set, but hole offset %u length %u block image length %u at %X/%X",
@@ -2099,6 +2104,21 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page)
bkpb = &record->record->blocks[block_id];
ptr = bkpb->bkp_image;
+ /*+ * The hole must fit in the page. DecodeXLogRecord() already enforces+ * this; re-check here before using the values as memcpy/MemSet lengths+ * or as the decompressor output capacity.+ */+ if (bkpb->hole_offset > BLCKSZ ||+ bkpb->hole_length > BLCKSZ - bkpb->hole_offset)+ {+ report_invalid_record(record,+ "could not restore image at %X/%X with invalid state, block %d",+ LSN_FORMAT_ARGS(record->ReadRecPtr),+ block_id);+ return false;+ }+
if (BKPIMAGE_COMPRESSED(bkpb->bimg_info))
{
/* If a backup block image is compressed, decompress it */
--
2.39.2
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-hackers@postgresql.org
Cc: ju.grigorev@ftdata.ru, pgsql-hackers@lists.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: <cc0a5886d2a74f99ae58c1647ea5ed8b@localhost.localdomain>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox