From 8b81786b31c1b94ae248a8d0235e79d94ed6ac01 Mon Sep 17 00:00:00 2001
From: Zexin Li <lizi.openmind@gmail.com>
Date: Tue, 4 Aug 2026 06:01:51 +0000
Subject: [PATCH] Reject invalid WAL locations in pg_waldump's -s/-e options

pg_waldump parsed its --start/--end arguments with sscanf("%X/%08X"),
which accepts several forms of input that the backend's pg_lsn type
rejects, and in each case proceeds with a location the user did not
specify:

* A first component wider than 32 bits overflows its uint32 argument,
  which is undefined behavior per C99 7.19.6.2p10; glibc keeps the
  low-order 32 bits, so -s 123456789/0 runs with 23456789/0.  A
  component wider than 64 bits additionally saturates to ULONG_MAX
  before the truncation.
* sscanf() succeeds without consuming the whole string, so trailing
  characters are silently ignored: 0/123456789 is read as 0/12345678
  and 1/2/3 as 1/2.
* The %X conversion follows strtoul()'s rules, so leading whitespace,
  signs, and "0x" prefixes are accepted: -s -1/0 runs with FFFFFFFF/0.

The eventual "is not inside file" error then reports the mangled
location rather than the given one, which is actively misleading.

Replace the sscanf() calls with a helper that follows the backend's
pg_lsn_in_safe(): one to eight hex digits, a slash, one to eight hex
digits, and nothing else.  Inputs the server accepts as pg_lsn are
accepted unchanged; everything else now fails with the existing
"invalid WAL location" error.

Add regression tests for the previously-accepted forms.

Bug: #19598
Reported-by: Michael Malis <malis@pgrust.com>
Discussion: https://postgr.es/m/19598-aa67c8f4331611b4@postgresql.org
---
 src/bin/pg_waldump/pg_waldump.c   | 41 +++++++++++++++++++++++++------
 src/bin/pg_waldump/t/001_basic.pl | 16 ++++++++++++
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index ffe6e8a6bc..9bb6031610 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -88,6 +88,38 @@ sigint_handler(SIGNAL_ARGS)
 }
 #endif
 
+#define MAXPG_LSNCOMPONENT	8
+
+/*
+ * Parse an LSN in the "%X/%X" format used for pg_lsn values, requiring
+ * one to eight hex digits in each component and nothing else, as the
+ * backend's pg_lsn_in_safe() does.  sscanf() is not strict enough here:
+ * its %X conversion has no field-width bound, so a component wider than
+ * 32 bits silently overflows the uint32 argument, and it also accepts
+ * leading whitespace, signs, "0x" prefixes, and trailing garbage.
+ *
+ * Returns true and sets *lsn on success.
+ */
+static bool
+parse_wal_location(const char *str, XLogRecPtr *lsn)
+{
+	int			len1,
+				len2;
+
+	len1 = strspn(str, "0123456789abcdefABCDEF");
+	if (len1 < 1 || len1 > MAXPG_LSNCOMPONENT || str[len1] != '/')
+		return false;
+
+	len2 = strspn(str + len1 + 1, "0123456789abcdefABCDEF");
+	if (len2 < 1 || len2 > MAXPG_LSNCOMPONENT || str[len1 + 1 + len2] != '\0')
+		return false;
+
+	*lsn = ((uint64) strtoul(str, NULL, 16)) << 32 |
+		(uint32) strtoul(str + len1 + 1, NULL, 16);
+
+	return true;
+}
+
 static void
 print_rmgr_list(void)
 {
@@ -928,8 +960,6 @@ usage(void)
 int
 main(int argc, char **argv)
 {
-	uint32		xlogid;
-	uint32		xrecoff;
 	XLogReaderState *xlogreader_state;
 	XLogDumpPrivate private;
 	XLogDumpConfig config;
@@ -1047,13 +1077,12 @@ main(int argc, char **argv)
 				config.filter_by_extended = true;
 				break;
 			case 'e':
-				if (sscanf(optarg, "%X/%08X", &xlogid, &xrecoff) != 2)
+				if (!parse_wal_location(optarg, &private.endptr))
 				{
 					pg_log_error("invalid WAL location: \"%s\"",
 								 optarg);
 					goto bad_argument;
 				}
-				private.endptr = (uint64) xlogid << 32 | xrecoff;
 				break;
 			case 'f':
 				config.follow = true;
@@ -1145,14 +1174,12 @@ main(int argc, char **argv)
 				config.filter_by_extended = true;
 				break;
 			case 's':
-				if (sscanf(optarg, "%X/%08X", &xlogid, &xrecoff) != 2)
+				if (!parse_wal_location(optarg, &private.startptr))
 				{
 					pg_log_error("invalid WAL location: \"%s\"",
 								 optarg);
 					goto bad_argument;
 				}
-				else
-					private.startptr = (uint64) xlogid << 32 | xrecoff;
 				break;
 			case 't':
 
diff --git a/src/bin/pg_waldump/t/001_basic.pl b/src/bin/pg_waldump/t/001_basic.pl
index 53b2f016b8..4fa507cfa2 100644
--- a/src/bin/pg_waldump/t/001_basic.pl
+++ b/src/bin/pg_waldump/t/001_basic.pl
@@ -54,6 +54,22 @@ command_fails_like(
 	[ 'pg_waldump', '--end' => 'bad' ],
 	qr/error: invalid WAL location/,
 	'invalid end LSN');
+command_fails_like(
+	[ 'pg_waldump', '--start' => '123456789/0' ],
+	qr/error: invalid WAL location/,
+	'start LSN with first component wider than 32 bits');
+command_fails_like(
+	[ 'pg_waldump', '--start' => '0/123456789' ],
+	qr/error: invalid WAL location/,
+	'start LSN with second component wider than 32 bits');
+command_fails_like(
+	[ 'pg_waldump', '--end' => '1/2/3' ],
+	qr/error: invalid WAL location/,
+	'end LSN with trailing garbage');
+command_fails_like(
+	[ 'pg_waldump', '--end' => '0x1/0' ],
+	qr/error: invalid WAL location/,
+	'end LSN with 0x prefix');
 
 # rmgr list: If you add one to the list, consider also adding a test
 # case exercising the new rmgr below.
-- 
2.34.1

