From 2ccf6f207a14dafe1a426208618ae7690c4513a2 Mon Sep 17 00:00:00 2001 From: David Steele Date: Thu, 24 Apr 2025 15:11:51 +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 | 17 +++++-- src/test/recovery/t/003_recovery_targets.pl | 50 +++++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index 6ce979f2d8b..e82aa739d79 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -4994,13 +4994,24 @@ check_recovery_target_timeline(char **newval, void **extra, GucSource source) rttg = RECOVERY_TARGET_TIMELINE_LATEST; else { + char *endp; + uint64 timeline; rttg = RECOVERY_TARGET_TIMELINE_NUMERIC; errno = 0; - strtoul(*newval, NULL, 0); - if (errno == EINVAL || errno == ERANGE) + timeline = strtou64(*newval, &endp, 0); + + if (*endp != '\0' || errno == EINVAL || errno == ERANGE) + { + GUC_check_errdetail("\"%s\" is not a valid number.", + "recovery_target_timeline"); + return false; + } + + if (timeline < 1 || timeline > UINT_MAX) { - GUC_check_errdetail("\"recovery_target_timeline\" is not a valid number."); + GUC_check_errdetail("\"%s\" must be between %u and %u.", + "recovery_target_timeline", 1, UINT_MAX); return false; } } diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl index 0ae2e982727..bd701d04f99 100644 --- a/src/test/recovery/t/003_recovery_targets.pl +++ b/src/test/recovery/t/003_recovery_targets.pl @@ -187,4 +187,54 @@ 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'); + +my $log_start = $node_standby->wait_for_log("is not a valid number"); + +# Timeline target out of min range +$node_standby->append_conf( + 'postgresql.conf', "recovery_target_timeline = '0'"); + +$res = run_log( + [ + 'pg_ctl', + '--pgdata' => $node_standby->data_dir, + '--log' => $node_standby->logfile, + 'start', + ]); +ok(!$res, 'invalid recovery startup fails'); + +$log_start = $node_standby->wait_for_log("must be between 1 and 4294967295", + $log_start); + +# Timeline target out of max range +$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'); + +$log_start = $node_standby->wait_for_log("must be between 1 and 4294967295", + $log_start); + done_testing(); -- 2.34.1