From 453c8937868e0c70e4bfecf0a39bf8052e379efe Mon Sep 17 00:00:00 2001
From: David Steele <david@pgmasters.net>
Date: Thu, 23 Jan 2025 14:40:43 +0000
Subject: Improve verification of recovery_target_timeline GUC.

Currently check_recovery_target_timeline() converts any value that is not
current, latest, or a valid integer to 0. So for example:

recovery_target_timeline = 'currrent'

results in the following error:

FATAL:  22023: recovery target timeline 0 does not exist

Since there is no range checking for uint32 (but there is a cast from unsigned long) this setting:

recovery_target_timeline = '9999999999'

results in the following error:

FATAL:  22023: recovery target timeline 1410065407 does not exist

Improve by adding endptr checking to catch conversion errors and add range checking to
exclude values < 2 and greater than UINT_MAX.
---
 src/backend/access/transam/xlogrecovery.c   | 14 ++++-
 src/test/recovery/t/003_recovery_targets.pl | 60 +++++++++++++++++++++
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index cf2b007806f..1941e3061f4 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -4973,15 +4973,25 @@ check_recovery_target_timeline(char **newval, void **extra, GucSource source)
 		rttg = RECOVERY_TARGET_TIMELINE_LATEST;
 	else
 	{
+		char		 *endp;
+		unsigned long long timeline;
 		rttg = RECOVERY_TARGET_TIMELINE_NUMERIC;
 
 		errno = 0;
-		strtoul(*newval, NULL, 0);
-		if (errno == EINVAL || errno == ERANGE)
+		timeline = strtoull(*newval, &endp, 0);
+
+		if (*endp != '\0' || errno == EINVAL || errno == ERANGE)
 		{
 			GUC_check_errdetail("\"recovery_target_timeline\" is not a valid number.");
 			return false;
 		}
+
+		if (timeline < 2 || timeline > UINT_MAX)
+		{
+			GUC_check_errdetail(
+				"\"recovery_target_timeline\" must be between 2 and 4,294,967,295.");
+			return false;
+		}
 	}
 
 	myextra = (RecoveryTargetTimeLineGoal *) guc_malloc(ERROR, sizeof(RecoveryTargetTimeLineGoal));
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 0ae2e982727..0424ff4ad22 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -187,4 +187,64 @@ ok( $logfile =~
 	  qr/FATAL: .* recovery ended before configured recovery target was reached/,
 	'recovery end before target reached is a fatal error');
 
+# Invalid timeline target
+$node_standby = PostgreSQL::Test::Cluster->new('standby_9');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+	has_restoring => 1);
+$node_standby->append_conf(
+	'postgresql.conf', "recovery_target_timeline = 'bogus'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid recovery startup fails');
+
+$logfile = slurp_file($node_standby->logfile());
+ok($logfile =~ qr/is not a valid number/,
+	'invalid target timelime');
+
+# Timeline target out of min range
+$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+	has_restoring => 1);
+$node_standby->append_conf(
+	'postgresql.conf', "recovery_target_timeline = '1'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid recovery startup fails');
+
+$logfile = slurp_file($node_standby->logfile());
+ok($logfile =~ qr/must be between 2 and 4,294,967,295/,
+	'target timelime out of min range');
+
+# Timeline target out of max range
+$node_standby = PostgreSQL::Test::Cluster->new('standby_11');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+	has_restoring => 1);
+$node_standby->append_conf(
+	'postgresql.conf', "recovery_target_timeline = '4294967296'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid recovery startup fails');
+
+$logfile = slurp_file($node_standby->logfile());
+ok($logfile =~ qr/must be between 2 and 4,294,967,295/,
+	'target timelime out of max range');
+
 done_testing();
-- 
2.34.1