BUG #19599: RestoreBlockImage: the decode cross-checks never bound hole_offset + hole_length against BLCKSZ

The following bug has been logged on the website:

Bug reference: 19599
Logged by: Michael Malis
Email address: malis(at)pgrust(dot)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.