Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Subject: BUG #19599: RestoreBlockImage: the decode cross-checks never bound
 hole_offset + hole_length against BLCKSZ
To: pgsql-bugs@lists.postgresql.org
From: PG Bug reporting form <noreply@postgresql.org>
Cc: malis@pgrust.com
Reply-To: malis@pgrust.com, pgsql-bugs@lists.postgresql.org
Date: Sun, 02 Aug 2026 17:51:51 +0000
Message-ID: <19599-8859c3822a831331@postgresql.org>
Auto-Submitted: auto-generated
Archived-At: 
 <https://www.postgresql.org/message-id/19599-8859c3822a831331%40postgresql.org>
Precedence: bulk

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:       =20

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 =E2=80=94 each is a=
 uint16,
so up to 65535 =E2=80=94 passes validation, and RestoreBlockImage then uses=
 those
values directly as memcpy/MemSet offsets and lengths into an 8 kB page
buffer.

Evidence status =E2=80=94 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 =E2=80=94 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    =3D 16
    hole_offset =3D 8000
    hole_length =3D 8000        (8000 + 8000 =3D 16000 > BLCKSZ 8192)
    bimg_info   =3D 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 =E2=80=94 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 =3D=3D 0 ||
    1832:       blk->hole_length =3D=3D 0 ||
    1833:       blk->bimg_len =3D=3D 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 =3D 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.