Re: BUG #19599: RestoreBlockImage: the decode cross-checks never bound hole_offset + hole_length against BLCKSZ
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253674psql -h localhost -U postgresBuilt from patchset v1 (message #1), September 09, 2026 at 08:22 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253674_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253674_1 && git checkout t253674_1Patchset v1 (message #1) is on t253674_1
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