From 4d28b9e2e68ce0561111d770218c9d80e869039a Mon Sep 17 00:00:00 2001
From: Zexin Li <lizi.openmind@gmail.com>
Date: Fri, 14 Aug 2026 00:37:28 +0000
Subject: [PATCH v1] Use pg_parse_lsn() for server-supplied LSNs

Commit d6bf0ab170 introduced pg_parse_lsn() to validate LSNs given on
the command line of pg_waldump, pg_recvlogical, and pg_receivewal.
The remaining frontend sscanf("%X/%08X") call sites parse LSNs that
arrive in server responses, timeline history files, and backup_label
files.  sscanf() accepts several forms that pg_lsn input rejects and
can silently continue with a different location than the input text:
a first component wider than eight hex digits wraps around, a wider
second component is truncated, and leading whitespace, signs, "0x"
prefixes, and trailing characters are consumed or ignored.

Convert those call sites as well.  Call sites that read a location
out of a longer line need to keep parsing where the location ended,
so add pg_parse_lsn_prefix(), which reports the first character after
the location instead of requiring the string to end there, and
reimplement pg_parse_lsn() on top of it.  Each tool keeps its
existing error message.

Malformed metadata now fails with each tool's existing error instead
of silently proceeding with a different location.  Two error-path
details change: pg_rewind's history-file parser now requires the
switchpoint to be followed by whitespace or end of line, where
trailing characters used to be ignored, and an overlong second
component in a backup_label LSN now fails pg_combinebackup's
"could not parse" check rather than its "improper terminator" check.

Author: Zexin Li <lizi.openmind@gmail.com>
---
 src/bin/pg_basebackup/pg_basebackup.c       |  16 +---
 src/bin/pg_basebackup/receivelog.c          |   8 +-
 src/bin/pg_basebackup/streamutil.c          |  24 ++---
 src/bin/pg_combinebackup/backup_label.c     |  12 +--
 src/bin/pg_combinebackup/t/005_integrity.pl |  26 +++++
 src/bin/pg_rewind/libpq_source.c            |   7 +-
 src/bin/pg_rewind/meson.build               |   1 +
 src/bin/pg_rewind/t/012_timeline_history.pl | 100 ++++++++++++++++++++
 src/bin/pg_rewind/timeline.c                |  22 +++--
 src/common/pg_parse_lsn.c                   |  53 +++++++++--
 src/include/common/pg_parse_lsn.h           |   2 +
 11 files changed, 206 insertions(+), 65 deletions(-)
 create mode 100644 src/bin/pg_rewind/t/012_timeline_history.pl

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 12fc752bff5..c3b87a19e76 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -32,6 +32,7 @@
 #include "common/file_perm.h"
 #include "common/file_utils.h"
 #include "common/logging.h"
+#include "common/pg_parse_lsn.h"
 #include "fe_utils/option_utils.h"
 #include "fe_utils/recovery_gen.h"
 #include "getopt_long.h"
@@ -482,17 +483,14 @@ reached_end_position(XLogRecPtr segendpos, uint32 timeline,
 		{
 			ssize_t		nread;
 			char		xlogend[64] = {0};
-			uint32		hi,
-						lo;
 
 			nread = read(bgpipe[0], xlogend, sizeof(xlogend) - 1);
 			if (nread < 0)
 				pg_fatal("could not read from ready pipe: %m");
 
-			if (sscanf(xlogend, "%X/%08X", &hi, &lo) != 2)
+			if (!pg_parse_lsn(xlogend, &xlogendptr))
 				pg_fatal("could not parse write-ahead log location \"%s\"",
 						 xlogend);
-			xlogendptr = ((uint64) hi) << 32 | lo;
 			has_xlogendptr = 1;
 
 			/*
@@ -620,8 +618,6 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier,
 				 int wal_compress_level)
 {
 	logstreamer_param *param;
-	uint32		hi,
-				lo;
 	char		statusdir[MAXPGPATH];
 
 	param = pg_malloc0_object(logstreamer_param);
@@ -631,10 +627,9 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier,
 	param->wal_compress_level = wal_compress_level;
 
 	/* Convert the starting position */
-	if (sscanf(startpos, "%X/%08X", &hi, &lo) != 2)
+	if (!pg_parse_lsn(startpos, &param->startptr))
 		pg_fatal("could not parse write-ahead log location \"%s\"",
 				 startpos);
-	param->startptr = ((uint64) hi) << 32 | lo;
 	/* Round off to even segment position */
 	param->startptr -= XLogSegmentOffset(param->startptr, WalSegSz);
 
@@ -2216,8 +2211,6 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
 		 * casting to a different size on WIN64.
 		 */
 		intptr_t	bgchild_handle = bgchild;
-		uint32		hi,
-					lo;
 #endif
 
 		if (verbose)
@@ -2243,10 +2236,9 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
 		 * value directly in the variable, and then set the flag that says
 		 * it's there.
 		 */
-		if (sscanf(xlogend, "%X/%08X", &hi, &lo) != 2)
+		if (!pg_parse_lsn(xlogend, &xlogendptr))
 			pg_fatal("could not parse write-ahead log location \"%s\"",
 					 xlogend);
-		xlogendptr = ((uint64) hi) << 32 | lo;
 		InterlockedIncrement(&has_xlogendptr);
 
 		/* First wait for the thread to exit */
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index faa60711b1b..77a2b4458b3 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -20,6 +20,7 @@
 
 #include "access/xlog_internal.h"
 #include "common/logging.h"
+#include "common/pg_parse_lsn.h"
 #include "libpq-fe.h"
 #include "libpq/protocol.h"
 #include "receivelog.h"
@@ -704,9 +705,6 @@ error:
 static bool
 ReadEndOfStreamingResult(PGresult *res, XLogRecPtr *startpos, uint32 *timeline)
 {
-	uint32		startpos_xlogid,
-				startpos_xrecoff;
-
 	/*----------
 	 * The result set consists of one row and two columns, e.g:
 	 *
@@ -727,14 +725,12 @@ ReadEndOfStreamingResult(PGresult *res, XLogRecPtr *startpos, uint32 *timeline)
 	}
 
 	*timeline = atoi(PQgetvalue(res, 0, 0));
-	if (sscanf(PQgetvalue(res, 0, 1), "%X/%08X", &startpos_xlogid,
-			   &startpos_xrecoff) != 2)
+	if (!pg_parse_lsn(PQgetvalue(res, 0, 1), startpos))
 	{
 		pg_log_error("could not parse next timeline's starting point \"%s\"",
 					 PQgetvalue(res, 0, 1));
 		return false;
 	}
-	*startpos = ((uint64) startpos_xlogid << 32) | startpos_xrecoff;
 
 	return true;
 }
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 8fcd690f155..e3c923cd317 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -21,6 +21,7 @@
 #include "common/connect.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/pg_parse_lsn.h"
 #include "common/string.h"
 #include "datatype/timestamp.h"
 #include "port/pg_bswap.h"
@@ -410,8 +411,6 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
 				  XLogRecPtr *startpos, char **db_name)
 {
 	PGresult   *res;
-	uint32		hi,
-				lo;
 
 	/* Check connection existence */
 	Assert(conn != NULL);
@@ -445,7 +444,7 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
 	/* Get LSN start position if necessary */
 	if (startpos != NULL)
 	{
-		if (sscanf(PQgetvalue(res, 0, 2), "%X/%08X", &hi, &lo) != 2)
+		if (!pg_parse_lsn(PQgetvalue(res, 0, 2), startpos))
 		{
 			pg_log_error("could not parse write-ahead log location \"%s\"",
 						 PQgetvalue(res, 0, 2));
@@ -453,7 +452,6 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
 			PQclear(res);
 			return false;
 		}
-		*startpos = ((uint64) hi) << 32 | lo;
 	}
 
 	/* Get database name, only available in 9.4 and newer versions */
@@ -547,19 +545,13 @@ GetSlotInformation(PGconn *conn, const char *slot_name,
 	}
 
 	/* restart LSN */
-	if (!PQgetisnull(res, 0, 1))
+	if (!PQgetisnull(res, 0, 1) &&
+		!pg_parse_lsn(PQgetvalue(res, 0, 1), &lsn_loc))
 	{
-		uint32		hi,
-					lo;
-
-		if (sscanf(PQgetvalue(res, 0, 1), "%X/%08X", &hi, &lo) != 2)
-		{
-			pg_log_error("could not parse restart_lsn \"%s\" for replication slot \"%s\"",
-						 PQgetvalue(res, 0, 1), slot_name);
-			PQclear(res);
-			return false;
-		}
-		lsn_loc = ((uint64) hi) << 32 | lo;
+		pg_log_error("could not parse restart_lsn \"%s\" for replication slot \"%s\"",
+					 PQgetvalue(res, 0, 1), slot_name);
+		PQclear(res);
+		return false;
 	}
 
 	/* current TLI */
diff --git a/src/bin/pg_combinebackup/backup_label.c b/src/bin/pg_combinebackup/backup_label.c
index b757e772b92..ca9ed2e7b4a 100644
--- a/src/bin/pg_combinebackup/backup_label.c
+++ b/src/bin/pg_combinebackup/backup_label.c
@@ -17,6 +17,7 @@
 #include "backup_label.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/pg_parse_lsn.h"
 #include "write_manifest.h"
 
 static int	get_eol_offset(StringInfo buf);
@@ -242,20 +243,15 @@ static bool
 parse_lsn(char *s, char *e, XLogRecPtr *lsn, char **c)
 {
 	char		save = *e;
-	int			nchars;
 	bool		success;
-	unsigned	hi;
-	unsigned	lo;
+	const char *end;
 
 	*e = '\0';
-	success = (sscanf(s, "%X/%08X%n", &hi, &lo, &nchars) == 2);
+	success = pg_parse_lsn_prefix(s, lsn, &end);
 	*e = save;
 
 	if (success)
-	{
-		*lsn = ((XLogRecPtr) hi) << 32 | (XLogRecPtr) lo;
-		*c = s + nchars;
-	}
+		*c = unconstify(char *, end);
 
 	return success;
 }
diff --git a/src/bin/pg_combinebackup/t/005_integrity.pl b/src/bin/pg_combinebackup/t/005_integrity.pl
index 9e1af2a7a7f..546d70c6d11 100644
--- a/src/bin/pg_combinebackup/t/005_integrity.pl
+++ b/src/bin/pg_combinebackup/t/005_integrity.pl
@@ -100,6 +100,7 @@ $node2->command_ok(
 
 # Result directory.
 my $resultpath = $node1->backup_dir . '/result';
+my $badlsnpath = $node1->backup_dir . '/badlsn';
 
 # Can't combine 2 full backups.
 $node1->command_fails_like(
@@ -208,5 +209,30 @@ $node1->command_fails_like(
 	qr/starts at LSN.*but expected/,
 	"can't combine synthetic backup with included incremental");
 
+# A start location whose first component is wider than 32 bits must be
+# rejected; it used to wrap around silently.
+my $labelpath = $backup2path . '/backup_label';
+my $origlabel = slurp_file($labelpath);
+my $badlabel = $origlabel;
+$badlabel =~
+  s{^START WAL LOCATION: [0-9A-F]+/}{START WAL LOCATION: 123456789/}m;
+open my $lfh, '>', $labelpath or die "$labelpath: $!";
+print $lfh $badlabel;
+close $lfh;
+$node1->command_fails_like(
+	[
+		'pg_combinebackup', $backup1path, $backup2path,
+		'--output' => $badlsnpath,
+		$mode,
+	],
+	qr/could not parse START WAL LOCATION/,
+	"can't combine a backup whose start location is out of range");
+rmtree($badlsnpath) if -d $badlsnpath;
+
+# Restore the original file.
+open $lfh, '>', $labelpath or die "$labelpath: $!";
+print $lfh $origlabel;
+close $lfh;
+
 # OK, that's all.
 done_testing();
diff --git a/src/bin/pg_rewind/libpq_source.c b/src/bin/pg_rewind/libpq_source.c
index 216755b3ddb..abfbd312580 100644
--- a/src/bin/pg_rewind/libpq_source.c
+++ b/src/bin/pg_rewind/libpq_source.c
@@ -11,6 +11,7 @@
 
 #include "catalog/pg_type_d.h"
 #include "common/connect.h"
+#include "common/pg_parse_lsn.h"
 #include "file_ops.h"
 #include "filemap.h"
 #include "lib/stringinfo.h"
@@ -209,17 +210,13 @@ libpq_get_current_wal_insert_lsn(rewind_source *source)
 {
 	PGconn	   *conn = ((libpq_source *) source)->conn;
 	XLogRecPtr	result;
-	uint32		hi;
-	uint32		lo;
 	char	   *val;
 
 	val = run_simple_query(conn, "SELECT pg_current_wal_insert_lsn()");
 
-	if (sscanf(val, "%X/%08X", &hi, &lo) != 2)
+	if (!pg_parse_lsn(val, &result))
 		pg_fatal("unrecognized result \"%s\" for current WAL insert location", val);
 
-	result = ((uint64) hi) << 32 | lo;
-
 	pg_free(val);
 
 	return result;
diff --git a/src/bin/pg_rewind/meson.build b/src/bin/pg_rewind/meson.build
index 52a6ab0a515..a328ab3e6b8 100644
--- a/src/bin/pg_rewind/meson.build
+++ b/src/bin/pg_rewind/meson.build
@@ -45,6 +45,7 @@ tests += {
       't/009_growing_files.pl',
       't/010_keep_recycled_wals.pl',
       't/011_wal_copy.pl',
+      't/012_timeline_history.pl',
     ],
   },
 }
diff --git a/src/bin/pg_rewind/t/012_timeline_history.pl b/src/bin/pg_rewind/t/012_timeline_history.pl
new file mode 100644
index 00000000000..f967dcfca4c
--- /dev/null
+++ b/src/bin/pg_rewind/t/012_timeline_history.pl
@@ -0,0 +1,100 @@
+
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+#
+# Test that a malformed switchpoint in a timeline history file is rejected
+# rather than silently misinterpreted.
+#
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+use FindBin;
+use lib $FindBin::RealBin;
+
+use RewindTest;
+
+RewindTest::setup_cluster('history');
+RewindTest::start_primary();
+RewindTest::create_standby('history');
+RewindTest::promote_standby();
+
+my $primary_pgdata = $node_primary->data_dir;
+my $standby_pgdata = $node_standby->data_dir;
+
+$node_standby->stop;
+$node_primary->stop;
+
+# The history file of the promoted standby is read by pg_rewind to find the
+# point where the two servers diverged.
+my $histfile = "$standby_pgdata/pg_wal/00000002.history";
+my $orig = slurp_file($histfile);
+
+sub write_history
+{
+	my $contents = shift;
+
+	open my $fh, '>', $histfile
+	  or BAIL_OUT("could not write \"$histfile\": $!");
+	print $fh $contents;
+	close $fh;
+	return;
+}
+
+# A switchpoint whose first component is wider than 32 bits must not be
+# accepted; the value used to wrap around silently.
+write_history("1\t123456789/0\tno recovery target specified\n");
+command_fails_like(
+	[
+		'pg_rewind',
+		'--dry-run',
+		'--source-pgdata' => $standby_pgdata,
+		'--target-pgdata' => $primary_pgdata,
+		'--no-sync',
+	],
+	qr/error: syntax error in history file/,
+	'switchpoint with first component wider than 32 bits');
+
+# Likewise for a second component that is too wide, which used to be
+# truncated to its first eight digits.
+write_history("1\t0/123456789\tno recovery target specified\n");
+command_fails_like(
+	[
+		'pg_rewind',
+		'--dry-run',
+		'--source-pgdata' => $standby_pgdata,
+		'--target-pgdata' => $primary_pgdata,
+		'--no-sync',
+	],
+	qr/error: syntax error in history file/,
+	'switchpoint with second component wider than 32 bits');
+
+# A switchpoint written with a "0x" prefix must also be rejected; sscanf's
+# %X used to consume the prefix silently.
+write_history("1\t0/0x3000000\tno recovery target specified\n");
+command_fails_like(
+	[
+		'pg_rewind',
+		'--dry-run',
+		'--source-pgdata' => $standby_pgdata,
+		'--target-pgdata' => $primary_pgdata,
+		'--no-sync',
+	],
+	qr/error: syntax error in history file/,
+	'switchpoint with 0x prefix');
+
+# The unmodified file is still accepted, so the failures above are caused by
+# the switchpoint and nothing else.
+write_history($orig);
+command_ok(
+	[
+		'pg_rewind',
+		'--dry-run',
+		'--source-pgdata' => $standby_pgdata,
+		'--target-pgdata' => $primary_pgdata,
+		'--no-sync',
+	],
+	'unmodified history file is accepted');
+
+done_testing();
diff --git a/src/bin/pg_rewind/timeline.c b/src/bin/pg_rewind/timeline.c
index dda06eaa0bc..957f1c88fcf 100644
--- a/src/bin/pg_rewind/timeline.c
+++ b/src/bin/pg_rewind/timeline.c
@@ -10,6 +10,7 @@
 #include "postgres_fe.h"
 
 #include "access/timeline.h"
+#include "common/pg_parse_lsn.h"
 #include "pg_rewind.h"
 
 /*
@@ -45,9 +46,9 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries)
 	{
 		char	   *ptr;
 		TimeLineID	tli;
-		uint32		switchpoint_hi;
-		uint32		switchpoint_lo;
-		int			nfields;
+		XLogRecPtr	switchpoint;
+		const char *lsnend;
+		int			nchars;
 
 		fline = bufptr;
 		while (*bufptr && *bufptr != '\n')
@@ -66,16 +67,21 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries)
 		if (*ptr == '\0' || *ptr == '#')
 			continue;
 
-		nfields = sscanf(fline, "%u\t%X/%08X", &tli, &switchpoint_hi, &switchpoint_lo);
-
-		if (nfields < 1)
+		if (sscanf(fline, "%u%n", &tli, &nchars) != 1)
 		{
 			/* expect a numeric timeline ID as first field of line */
 			pg_log_error("syntax error in history file: %s", fline);
 			pg_log_error_detail("Expected a numeric timeline ID.");
 			exit(1);
 		}
-		if (nfields != 3)
+
+		/* the switchpoint location follows, separated by whitespace */
+		ptr = fline + nchars;
+		while (isspace((unsigned char) *ptr))
+			ptr++;
+
+		if (!pg_parse_lsn_prefix(ptr, &switchpoint, &lsnend) ||
+			(*lsnend != '\0' && !isspace((unsigned char) *lsnend)))
 		{
 			pg_log_error("syntax error in history file: %s", fline);
 			pg_log_error_detail("Expected a write-ahead log switchpoint location.");
@@ -96,7 +102,7 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries)
 		entry = &entries[nlines - 1];
 		entry->tli = tli;
 		entry->begin = prevend;
-		entry->end = ((uint64) (switchpoint_hi)) << 32 | (uint64) switchpoint_lo;
+		entry->end = switchpoint;
 		prevend = entry->end;
 
 		/* we ignore the remainder of each line */
diff --git a/src/common/pg_parse_lsn.c b/src/common/pg_parse_lsn.c
index b6a83b57216..1979fa02387 100644
--- a/src/common/pg_parse_lsn.c
+++ b/src/common/pg_parse_lsn.c
@@ -23,20 +23,21 @@
 #define MAXPG_LSNCOMPONENT	8
 
 /*
- * pg_parse_lsn
+ * pg_parse_lsn_prefix
  *
  * Parse a WAL location in the "%X/%X" text form used for pg_lsn values,
- * requiring one to eight hexadecimal digits in each component and nothing
- * else.  Unlike sscanf(), this rejects components longer than eight
- * hexadecimal digits, leading whitespace, signs, "0x" prefixes, and
- * trailing characters.
+ * requiring one to eight hexadecimal digits in each component.  Unlike
+ * sscanf(), this rejects components longer than eight hexadecimal digits,
+ * leading whitespace, signs, and "0x" prefixes.
  *
- * Returns true and sets *result on success; returns false on syntax
- * error, leaving *result unchanged.
+ * Returns true on success, setting *result to the location and *endptr to
+ * the first character after it.  Returns false on syntax error, leaving
+ * both unchanged.
  */
 bool
-pg_parse_lsn(const char *str, XLogRecPtr *result)
+pg_parse_lsn_prefix(const char *str, XLogRecPtr *result, const char **endptr)
 {
+	char		buf[MAXPG_LSNCOMPONENT + 1];
 	size_t		len1,
 				len2;
 
@@ -45,11 +46,43 @@ pg_parse_lsn(const char *str, XLogRecPtr *result)
 		return false;
 
 	len2 = strspn(str + len1 + 1, "0123456789abcdefABCDEF");
-	if (len2 < 1 || len2 > MAXPG_LSNCOMPONENT || str[len1 + 1 + len2] != '\0')
+	if (len2 < 1 || len2 > MAXPG_LSNCOMPONENT)
 		return false;
 
+	/*
+	 * Decode the second component from a bounded copy: on the original
+	 * string, strtoul() would accept a "0x" prefix that the check above did
+	 * not.  (The first component ends at the '/'.)
+	 */
+	memcpy(buf, str + len1 + 1, len2);
+	buf[len2] = '\0';
+
 	*result = ((uint64) strtoul(str, NULL, 16)) << 32 |
-		(uint32) strtoul(str + len1 + 1, NULL, 16);
+		(uint32) strtoul(buf, NULL, 16);
+	*endptr = str + len1 + 1 + len2;
+
+	return true;
+}
+
+/*
+ * pg_parse_lsn
+ *
+ * Same as pg_parse_lsn_prefix(), but the whole string has to be a WAL
+ * location.
+ *
+ * Returns true and sets *result on success; returns false on syntax
+ * error, leaving *result unchanged.
+ */
+bool
+pg_parse_lsn(const char *str, XLogRecPtr *result)
+{
+	XLogRecPtr	lsn;
+	const char *end;
+
+	if (!pg_parse_lsn_prefix(str, &lsn, &end) || *end != '\0')
+		return false;
+
+	*result = lsn;
 
 	return true;
 }
diff --git a/src/include/common/pg_parse_lsn.h b/src/include/common/pg_parse_lsn.h
index 0a80785a4e9..740ce6e1226 100644
--- a/src/include/common/pg_parse_lsn.h
+++ b/src/include/common/pg_parse_lsn.h
@@ -16,5 +16,7 @@
 #include "access/xlogdefs.h"
 
 extern bool pg_parse_lsn(const char *str, XLogRecPtr *result);
+extern bool pg_parse_lsn_prefix(const char *str, XLogRecPtr *result,
+								const char **endptr);
 
 #endif							/* PG_PARSE_LSN_H */
-- 
2.34.1

