From 45cc3b63a3680066ab198b1a0141a9c9fde9a619 Mon Sep 17 00:00:00 2001
From: David karapetyan <dkarapetyan@gmail.com>
Date: Sat, 25 Jul 2026 13:32:48 -0700
Subject: [PATCH] Fix XLogReader mishandling of oversized multi-page records.

XLogRecordAssemble() refuses records larger than XLogRecordMaxSize, but
the reader only checked a minimum xl_tot_len. A crafted multi-page record
with xl_tot_len near UINT32_MAX could pass contrecord length checks,
overflow allocate_recordbuf()'s uint32 size math, and corrupt memory
during reassembly (or hit related asserts under cassert).

Cap xl_tot_len at XLogRecordMaxSize in ValidXLogRecordHeader() and on
the partial-header path before reassembly, and compute reassembly buffer
sizes with size_t. Add a frontend test that feeds crafted multi-page WAL
to the reader.
---
 src/backend/access/transam/xlogreader.c       |  53 +++-
 src/test/modules/Makefile                     |   1 +
 src/test/modules/meson.build                  |   1 +
 src/test/modules/test_xlogreader/Makefile     |  30 +++
 src/test/modules/test_xlogreader/README       |  35 +++
 src/test/modules/test_xlogreader/meson.build  |  34 +++
 .../test_xlogreader/t/001_oversized_record.pl |  35 +++
 .../test_xlogreader_oversized.c               | 234 ++++++++++++++++++
 8 files changed, 417 insertions(+), 6 deletions(-)
 create mode 100644 src/test/modules/test_xlogreader/Makefile
 create mode 100644 src/test/modules/test_xlogreader/README
 create mode 100644 src/test/modules/test_xlogreader/meson.build
 create mode 100644 src/test/modules/test_xlogreader/t/001_oversized_record.pl
 create mode 100644 src/test/modules/test_xlogreader/test_xlogreader_oversized.c

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 946907a..d1a7e38 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -186,20 +186,35 @@ XLogReaderFree(XLogReaderState *state)
  * abort records might need more space.)
  *
  * Note: This routine should *never* be called for xl_tot_len until the header
- * of the record has been fully validated.
+ * of the record has been fully validated (including the XLogRecordMaxSize
+ * bound).  Size math uses size_t so near-UINT32_MAX lengths cannot wrap to a
+ * small allocation.
  */
 static void
 allocate_recordbuf(XLogReaderState *state, uint32 reclength)
 {
-	uint32		newSize = reclength;
+	size_t		newSize = (size_t) reclength;
+	size_t		minsize = (size_t) 5 * Max(BLCKSZ, XLOG_BLCKSZ);
 
-	newSize += XLOG_BLCKSZ - (newSize % XLOG_BLCKSZ);
-	newSize = Max(newSize, 5 * Max(BLCKSZ, XLOG_BLCKSZ));
+	/*
+	 * Round up to a multiple of XLOG_BLCKSZ.  When already aligned (including
+	 * zero), advance by a full page — same as the historical uint32 formula.
+	 */
+	newSize += (size_t) XLOG_BLCKSZ - (newSize % (size_t) XLOG_BLCKSZ);
+	if (newSize < minsize)
+		newSize = minsize;
+
+	/*
+	 * Callers must reject xl_tot_len > XLogRecordMaxSize first.  With that
+	 * bound, the rounded size always fits in uint32 (and palloc's limit).
+	 */
+	Assert(newSize <= (size_t) XLogRecordMaxSize + (size_t) XLOG_BLCKSZ);
+	Assert(newSize <= (size_t) PG_UINT32_MAX);
 
 	if (state->readRecordBuf)
 		pfree(state->readRecordBuf);
 	state->readRecordBuf = (char *) palloc(newSize);
-	state->readRecordBufSize = newSize;
+	state->readRecordBufSize = (uint32) newSize;
 }
 
 /*
@@ -664,7 +679,11 @@ restart:
 	}
 	else
 	{
-		/* There may be no next page if it's too small. */
+		/*
+		 * There may be no next page if it's too small.  Cap xl_tot_len before
+		 * contrecord reassembly so we never allocate or copy based on a
+		 * garbage length from a recycled page.
+		 */
 		if (total_len < SizeOfXLogRecord)
 		{
 			report_invalid_record(state,
@@ -673,6 +692,14 @@ restart:
 								  (uint32) SizeOfXLogRecord, total_len);
 			goto err;
 		}
+		if (total_len > XLogRecordMaxSize)
+		{
+			report_invalid_record(state,
+								  "invalid record length at %X/%08X: expected at most %u, got %u",
+								  LSN_FORMAT_ARGS(RecPtr),
+								  XLogRecordMaxSize, total_len);
+			goto err;
+		}
 		/* We'll validate the header once we have the next page. */
 		gotheader = false;
 	}
@@ -1148,6 +1175,20 @@ ValidXLogRecordHeader(XLogReaderState *state, XLogRecPtr RecPtr,
 							  (uint32) SizeOfXLogRecord, record->xl_tot_len);
 		return false;
 	}
+	/*
+	 * Symmetric with XLogRecordAssemble(): the reader must not attempt to
+	 * reassemble or decode a record larger than XLogRecordMaxSize.  Without
+	 * this bound, a crafted multi-page xl_tot_len near UINT32_MAX can overflow
+	 * allocate_recordbuf()'s size math and corrupt memory during reassembly.
+	 */
+	if (record->xl_tot_len > XLogRecordMaxSize)
+	{
+		report_invalid_record(state,
+							  "invalid record length at %X/%08X: expected at most %u, got %u",
+							  LSN_FORMAT_ARGS(RecPtr),
+							  XLogRecordMaxSize, record->xl_tot_len);
+		return false;
+	}
 	if (!RmgrIdIsValid(record->xl_rmid))
 	{
 		report_invalid_record(state,
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 098bb81..4b59eff 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -53,6 +53,7 @@ SUBDIRS = \
 		  test_shm_mq \
 		  test_slru \
 		  test_tidstore \
+		  test_xlogreader \
 		  unsafe_tests \
 		  worker_spi \
 		  xid_wraparound
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 4bca42b..effbaff 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -54,6 +54,7 @@ subdir('test_shmem')
 subdir('test_shm_mq')
 subdir('test_slru')
 subdir('test_tidstore')
+subdir('test_xlogreader')
 subdir('typcache')
 subdir('unsafe_tests')
 subdir('worker_spi')
diff --git a/src/test/modules/test_xlogreader/Makefile b/src/test/modules/test_xlogreader/Makefile
new file mode 100644
index 0000000..bf9f574
--- /dev/null
+++ b/src/test/modules/test_xlogreader/Makefile
@@ -0,0 +1,30 @@
+# src/test/modules/test_xlogreader/Makefile
+
+PGFILEDESC = "test_xlogreader - XLogReader validation tests"
+
+PROGRAM = test_xlogreader_oversized
+OBJS = $(WIN32RES) test_xlogreader_oversized.o xlogreader.o
+
+# Frontend build of backend xlogreader.c (same pattern as pg_waldump)
+override CPPFLAGS := -DFRONTEND $(CPPFLAGS)
+PG_CPPFLAGS = -I$(libpq_srcdir)
+PG_LIBS_INTERNAL += $(libpq_pgport)
+
+NO_INSTALL = 1
+TAP_TESTS = 1
+
+EXTRA_CLEAN = xlogreader.c
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_xlogreader
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
+
+xlogreader.c: % : $(top_srcdir)/src/backend/access/transam/%
+	rm -f $@ && $(LN_S) $< .
diff --git a/src/test/modules/test_xlogreader/README b/src/test/modules/test_xlogreader/README
new file mode 100644
index 0000000..02eecfc
--- /dev/null
+++ b/src/test/modules/test_xlogreader/README
@@ -0,0 +1,35 @@
+test_xlogreader
+===============
+
+Frontend tests for XLogReader validation of pathological multi-page WAL.
+
+test_xlogreader_oversized
+-------------------------
+
+Constructs an in-memory multi-page WAL record with:
+
+* xl_tot_len = 0xFFFFF000 (near UINT32_MAX, far above XLogRecordMaxSize)
+* matching XLP_FIRST_IS_CONTRECORD / xlp_rem_len on continuation pages
+
+and feeds it to XLogReader.
+
+Correct behavior
+  XLogReadRecord() returns NULL with an error; process exits 0.
+
+Buggy behavior (current mainline as of this test)
+  allocate_recordbuf() rounds the huge length with uint32 arithmetic and
+  wraps to a ~40kB buffer.  On the next continuation page, reassembly hits:
+
+    Assert(gotlen <= lengthof(save_copy));  /* save_copy is 2 * XLOG_BLCKSZ */
+
+  under USE_ASSERT_CHECKING (abort), or stack/heap overflow without asserts.
+
+Build / run
+-----------
+
+  make -C src/test/modules/test_xlogreader
+  ./src/test/modules/test_xlogreader/test_xlogreader_oversized
+
+With TAP enabled (--enable-tap-tests):
+
+  make -C src/test/modules/test_xlogreader check
diff --git a/src/test/modules/test_xlogreader/meson.build b/src/test/modules/test_xlogreader/meson.build
new file mode 100644
index 0000000..acc1e92
--- /dev/null
+++ b/src/test/modules/test_xlogreader/meson.build
@@ -0,0 +1,34 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+test_xlogreader_sources = files(
+  'test_xlogreader_oversized.c',
+)
+test_xlogreader_sources += xlogreader_sources
+
+if host_system == 'windows'
+  test_xlogreader_sources += rc_bin_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'test_xlogreader_oversized',
+    '--FILEDESC', 'test XLogReader oversized record handling',])
+endif
+
+test_xlogreader_oversized = executable('test_xlogreader_oversized',
+  test_xlogreader_sources,
+  dependencies: [frontend_code, libpq],
+  c_args: ['-DFRONTEND'],
+  kwargs: default_bin_args + {
+    'install': false,
+  },
+)
+testprep_targets += test_xlogreader_oversized
+
+tests += {
+  'name': 'test_xlogreader',
+  'sd': meson.current_source_dir(),
+  'bd': meson.current_build_dir(),
+  'tap': {
+    'tests': [
+      't/001_oversized_record.pl',
+    ],
+    'deps': [test_xlogreader_oversized],
+  },
+}
diff --git a/src/test/modules/test_xlogreader/t/001_oversized_record.pl b/src/test/modules/test_xlogreader/t/001_oversized_record.pl
new file mode 100644
index 0000000..ddde7d3
--- /dev/null
+++ b/src/test/modules/test_xlogreader/t/001_oversized_record.pl
@@ -0,0 +1,35 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Expect XLogReader to reject multi-page records with xl_tot_len near
+# UINT32_MAX (or above XLogRecordMaxSize) without crashing.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $exe = 'test_xlogreader_oversized';
+
+note "running $exe (expects clean rejection of oversized multi-page WAL)";
+
+# With the allocate_recordbuf overflow bug this often dies on SIGSEGV.
+# After a proper max-length check it exits 0 and prints OK.
+my $result = IPC::Run::run [ $exe ], '>', \my $stdout, '2>', \my $stderr;
+my $exit = $? >> 8;
+my $signal = $? & 127;
+
+if ($signal)
+{
+	fail("test_xlogreader_oversized died with signal $signal (likely heap overflow in allocate_recordbuf / reassembly)");
+	diag("stdout: $stdout") if length $stdout;
+	diag("stderr: $stderr") if length $stderr;
+}
+else
+{
+	is($exit, 0, 'test_xlogreader_oversized exits 0');
+	like($stdout, qr/^OK:/, 'reports clean rejection on stdout');
+	is($stderr, '', 'no stderr on success');
+}
+
+done_testing();
diff --git a/src/test/modules/test_xlogreader/test_xlogreader_oversized.c b/src/test/modules/test_xlogreader/test_xlogreader_oversized.c
new file mode 100644
index 0000000..43ba56f
--- /dev/null
+++ b/src/test/modules/test_xlogreader/test_xlogreader_oversized.c
@@ -0,0 +1,234 @@
+/*-------------------------------------------------------------------------
+ *
+ * test_xlogreader_oversized.c
+ *	  Demonstrate that XLogReader fails to reject multi-page records whose
+ *	  xl_tot_len exceeds XLogRecordMaxSize, and that allocate_recordbuf()
+ *	  can overflow uint32 when rounding a near-UINT32_MAX length.
+ *
+ *	  Crafted multi-page WAL is fed through the frontend XLogReader.  A
+ *	  correct reader must reject the record cleanly (return NULL with an
+ *	  error).  Unfixed code reallocates with a wrapped size (~40kB) and
+ *	  then heap-overflows while reassembling continuation pages.
+ *
+ *	  Build and run (from a configured tree):
+ *	    make -C src/test/modules/test_xlogreader
+ *	    ./src/test/modules/test_xlogreader/test_xlogreader_oversized
+ *
+ *	  Expected with the bug:
+ *	    - with asserts: abort on
+ *	        Assert(gotlen <= lengthof(save_copy)) in XLogDecodeNextRecord
+ *	        (gotlen exceeds 2 pages while total_len still forces reallocation)
+ *	    - without asserts: stack/heap overflow around save_copy /
+ *	        allocate_recordbuf uint32 wrap
+ *	  Expected after fix: exit 0 and a rejection message on stdout.
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres_fe.h"
+
+#include <string.h>
+
+#include "access/xlog_internal.h"
+#include "access/xlogreader.h"
+#include "access/xlogrecord.h"
+#include "common/fe_memutils.h"
+
+/* Default segment size; only used for page-header cross-checks. */
+#define TEST_WAL_SEGSIZE	(16 * 1024 * 1024)
+
+/*
+ * Enough pages for reassembly to reallocate and then overflow a 40kB
+ * buffer: first page + a few continuations.  total_len claims ~4GB so
+ * the reader keeps consuming pages until it corrupts memory (bug) or
+ * rejects (fixed).
+ */
+#define TEST_NPAGES			16
+
+/* Near UINT32_MAX so allocate_recordbuf() rounding wraps to 0. */
+#define TEST_XL_TOT_LEN		((uint32) 0xFFFFF000U)
+
+typedef struct TestWalState
+{
+	char		pages[TEST_NPAGES][XLOG_BLCKSZ];
+	int			npages;
+} TestWalState;
+
+static void
+init_short_page_header(char *page, XLogRecPtr pageaddr, uint16 info,
+					   uint32 rem_len)
+{
+	XLogPageHeader hdr = (XLogPageHeader) page;
+
+	memset(page, 0, XLOG_BLCKSZ);
+	hdr->xlp_magic = XLOG_PAGE_MAGIC;
+	hdr->xlp_info = info;
+	hdr->xlp_tli = 1;
+	hdr->xlp_pageaddr = pageaddr;
+	hdr->xlp_rem_len = rem_len;
+}
+
+static void
+init_long_page_header(char *page, XLogRecPtr pageaddr, uint16 info,
+					  uint32 rem_len)
+{
+	XLogLongPageHeader longhdr = (XLogLongPageHeader) page;
+
+	init_short_page_header(page, pageaddr, info | XLP_LONG_HEADER, rem_len);
+	longhdr->xlp_sysid = 0;
+	longhdr->xlp_seg_size = TEST_WAL_SEGSIZE;
+	longhdr->xlp_xlog_blcksz = XLOG_BLCKSZ;
+}
+
+/*
+ * Build a multi-page record starting at the first data byte after the long
+ * header on page 0.  xl_tot_len is huge; continuation pages advertise
+ * matching xlp_rem_len values so the reader enters reassembly.
+ */
+static XLogRecPtr
+build_oversized_multpage_wal(TestWalState *ws)
+{
+	XLogRecPtr	page0 = 0;
+	XLogRecPtr	recptr;
+	XLogRecord *rec;
+	char	   *payload;
+	uint32		gotlen;
+	int			i;
+	int			phd_long = SizeOfXLogLongPHD;
+	int			phd_short = SizeOfXLogShortPHD;
+
+	ws->npages = TEST_NPAGES;
+
+	/* Page 0: long header + start of record */
+	init_long_page_header(ws->pages[0], page0, 0, 0);
+	recptr = page0 + phd_long;
+	payload = ws->pages[0] + phd_long;
+
+	/* Zero-fill usable area, then write the fixed-size record header. */
+	memset(payload, 0, XLOG_BLCKSZ - phd_long);
+	rec = (XLogRecord *) payload;
+	rec->xl_tot_len = TEST_XL_TOT_LEN;
+	rec->xl_xid = 0;
+	rec->xl_prev = 0;			/* randAccess accepts xl_prev < recptr */
+	rec->xl_info = 0;
+	rec->xl_rmid = RM_XLOG_ID;
+	rec->xl_crc = 0;			/* CRC never reached on the overflow path */
+
+	gotlen = XLOG_BLCKSZ - phd_long;
+
+	/* Continuation pages with consistent xlp_rem_len */
+	for (i = 1; i < TEST_NPAGES; i++)
+	{
+		XLogRecPtr	pageaddr = (XLogRecPtr) i * XLOG_BLCKSZ;
+		uint32		rem = TEST_XL_TOT_LEN - gotlen;
+		int			usable = XLOG_BLCKSZ - phd_short;
+
+		init_short_page_header(ws->pages[i], pageaddr,
+							   XLP_FIRST_IS_CONTRECORD, rem);
+		memset(ws->pages[i] + phd_short, 0xAB, usable);
+		gotlen += usable;
+	}
+
+	return recptr;
+}
+
+static int
+test_page_read(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
+			   int reqLen, XLogRecPtr targetRecPtr, char *readBuf)
+{
+	TestWalState *ws = (TestWalState *) xlogreader->private_data;
+	uint64		idx;
+
+	(void) reqLen;
+	(void) targetRecPtr;
+
+	if (targetPagePtr % XLOG_BLCKSZ != 0)
+		return -1;
+
+	idx = targetPagePtr / XLOG_BLCKSZ;
+	if (idx >= (uint64) ws->npages)
+		return -1;
+
+	memcpy(readBuf, ws->pages[idx], XLOG_BLCKSZ);
+	xlogreader->seg.ws_tli = 1;
+	return XLOG_BLCKSZ;
+}
+
+static void
+test_segment_open(XLogReaderState *xlogreader, XLogSegNo nextSegNo,
+				  TimeLineID *tli_p)
+{
+	(void) xlogreader;
+	(void) nextSegNo;
+	/* Keep TLI from caller / page_read. */
+	if (tli_p && *tli_p == 0)
+		*tli_p = 1;
+	xlogreader->seg.ws_file = 0;	/* dummy non -1 */
+	xlogreader->seg.ws_segno = nextSegNo;
+	xlogreader->seg.ws_tli = 1;
+}
+
+static void
+test_segment_close(XLogReaderState *xlogreader)
+{
+	xlogreader->seg.ws_file = -1;
+}
+
+int
+main(int argc, char **argv)
+{
+	TestWalState ws;
+	XLogReaderState *state;
+	XLogReaderRoutine routine;
+	XLogRecPtr	start;
+	XLogRecord *record;
+	char	   *errormsg = NULL;
+
+	(void) argc;
+	(void) argv;
+
+	memset(&ws, 0, sizeof(ws));
+	start = build_oversized_multpage_wal(&ws);
+
+	memset(&routine, 0, sizeof(routine));
+	routine.page_read = test_page_read;
+	routine.segment_open = test_segment_open;
+	routine.segment_close = test_segment_close;
+
+	state = XLogReaderAllocate(TEST_WAL_SEGSIZE, NULL, &routine, &ws);
+	if (state == NULL)
+	{
+		fprintf(stderr, "out of memory allocating XLogReader\n");
+		return 2;
+	}
+
+	/*
+	 * Begin at the crafted record.  A fixed reader must refuse xl_tot_len
+	 * above XLogRecordMaxSize (or fail closed on size overflow) without
+	 * scribbling past its reassembly buffer.
+	 */
+	XLogBeginRead(state, start);
+	record = XLogReadRecord(state, &errormsg);
+
+	if (record != NULL)
+	{
+		fprintf(stderr,
+				"FAIL: oversized multi-page record was accepted "
+				"(xl_tot_len=%u XLogRecordMaxSize=%u)\n",
+				TEST_XL_TOT_LEN, (unsigned) XLogRecordMaxSize);
+		XLogReaderFree(state);
+		return 1;
+	}
+
+	if (errormsg == NULL || errormsg[0] == '\0')
+	{
+		fprintf(stderr,
+				"FAIL: reader returned NULL without an error message\n");
+		XLogReaderFree(state);
+		return 1;
+	}
+
+	/* Clean rejection — this is the desired post-fix behavior. */
+	printf("OK: rejected oversized record: %s\n", errormsg);
+	XLogReaderFree(state);
+	return 0;
+}
-- 
2.43.0

