From 7efc1573c1bcc07c0eaa80912e6e035f2e0d203d Mon Sep 17 00:00:00 2001
From: Pierre Ducroquet <p.psql@pinaraf.info>
Date: Wed, 13 Sep 2017 19:51:09 +0200
Subject: [PATCH] Port most calls of atoi(optarg) to strcol

atoi does not allow any kind of data check. If the input data is invalid,
the result will be silently truncated to the valid part of input, or just
0 if no digit was found at the beginning of input.
---
 src/bin/pg_basebackup/pg_basebackup.c  | 11 ++++++-----
 src/bin/pg_basebackup/pg_receivewal.c  | 11 ++++++-----
 src/bin/pg_basebackup/pg_recvlogical.c | 11 ++++++-----
 src/bin/pg_ctl/pg_ctl.c                |  9 ++++++++-
 src/bin/pg_dump/pg_dump.c              | 12 +++++++++---
 src/bin/pg_dump/pg_restore.c           |  8 +++++++-
 src/bin/pg_upgrade/option.c            |  5 +++--
 src/bin/pgbench/pgbench.c              | 33 +++++++++++++++++----------------
 8 files changed, 62 insertions(+), 38 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 51509d150e..b51b62cc21 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -87,7 +87,7 @@ static IncludeWal includewal = STREAM_WAL;
 static bool fastcheckpoint = false;
 static bool writerecoveryconf = false;
 static bool do_sync = true;
-static int	standby_message_timeout = 10 * 1000;	/* 10 sec = default */
+static long int	standby_message_timeout = 10 * 1000;	/* 10 sec = default */
 static pg_time_t last_progress_report = 0;
 static int32 maxrate = 0;		/* no limit by default */
 static char *replication_slot = NULL;
@@ -2072,6 +2072,7 @@ BaseBackup(void)
 int
 main(int argc, char **argv)
 {
+	char *strtol_endptr = NULL;
 	static struct option long_options[] = {
 		{"help", no_argument, NULL, '?'},
 		{"version", no_argument, NULL, 'V'},
@@ -2212,8 +2213,8 @@ main(int argc, char **argv)
 #endif
 				break;
 			case 'Z':
-				compresslevel = atoi(optarg);
-				if (compresslevel < 0 || compresslevel > 9)
+				compresslevel = strtol(optarg, &strtol_endptr, 10);
+				if (compresslevel < 0 || compresslevel > 9 || (strtol_endptr != optarg + strlen(optarg)))
 				{
 					fprintf(stderr, _("%s: invalid compression level \"%s\"\n"),
 							progname, optarg);
@@ -2251,8 +2252,8 @@ main(int argc, char **argv)
 				dbgetpassword = 1;
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				standby_message_timeout = strtol(optarg, &strtol_endptr, 10) * 1000;
+				if ((standby_message_timeout < 0) || (strtol_endptr != optarg + strlen(optarg)))
 				{
 					fprintf(stderr, _("%s: invalid status interval \"%s\"\n"),
 							progname, optarg);
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 710a33ab4d..c1651961b5 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -494,6 +494,7 @@ main(int argc, char **argv)
 	int			c;
 	int			option_index;
 	char	   *db_name;
+	char	   *strtol_endptr = NULL;
 	uint32		hi, lo;
 
 	progname = get_progname(argv[0]);
@@ -529,7 +530,7 @@ main(int argc, char **argv)
 				dbhost = pg_strdup(optarg);
 				break;
 			case 'p':
-				if (atoi(optarg) <= 0)
+				if ((strtol(optarg, &strtol_endptr, 10) <= 0) || (strtol_endptr != optarg + strlen(optarg)))
 				{
 					fprintf(stderr, _("%s: invalid port number \"%s\"\n"),
 							progname, optarg);
@@ -547,8 +548,8 @@ main(int argc, char **argv)
 				dbgetpassword = 1;
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				standby_message_timeout = strtol(optarg, &strtol_endptr, 10) * 1000;
+				if ((standby_message_timeout < 0) || (strtol_endptr != optarg + strlen(optarg)))
 				{
 					fprintf(stderr, _("%s: invalid status interval \"%s\"\n"),
 							progname, optarg);
@@ -575,8 +576,8 @@ main(int argc, char **argv)
 				verbose++;
 				break;
 			case 'Z':
-				compresslevel = atoi(optarg);
-				if (compresslevel < 0 || compresslevel > 9)
+				compresslevel = strtol(optarg, &strtol_endptr, 10);
+				if (compresslevel < 0 || compresslevel > 9 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, _("%s: invalid compression level \"%s\"\n"),
 							progname, optarg);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 6811a55e76..bde45b2650 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -706,6 +706,7 @@ main(int argc, char **argv)
 	uint32		hi,
 				lo;
 	char	   *db_name;
+	char	   *strtol_endptr = NULL;
 
 	progname = get_progname(argv[0]);
 	set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_basebackup"));
@@ -735,8 +736,8 @@ main(int argc, char **argv)
 				outfile = pg_strdup(optarg);
 				break;
 			case 'F':
-				fsync_interval = atoi(optarg) * 1000;
-				if (fsync_interval < 0)
+				fsync_interval = strtol(optarg, &strtol_endptr, 10) * 1000;
+				if (fsync_interval < 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, _("%s: invalid fsync interval \"%s\"\n"),
 							progname, optarg);
@@ -757,7 +758,7 @@ main(int argc, char **argv)
 				dbhost = pg_strdup(optarg);
 				break;
 			case 'p':
-				if (atoi(optarg) <= 0)
+				if (strtol(optarg, &strtol_endptr, 10) <= 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, _("%s: invalid port number \"%s\"\n"),
 							progname, optarg);
@@ -819,8 +820,8 @@ main(int argc, char **argv)
 				plugin = pg_strdup(optarg);
 				break;
 			case 's':
-				standby_message_timeout = atoi(optarg) * 1000;
-				if (standby_message_timeout < 0)
+				standby_message_timeout = strtol(optarg, &strtol_endptr, 10) * 1000;
+				if (standby_message_timeout < 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, _("%s: invalid status interval \"%s\"\n"),
 							progname, optarg);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 4e02c4cea1..2190412293 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2100,6 +2100,7 @@ main(int argc, char **argv)
 	};
 
 	char	   *env_wait;
+	char	   *strtol_endptr = NULL;
 	int			option_index;
 	int			c;
 	pgpid_t		killproc = 0;
@@ -2231,7 +2232,13 @@ main(int argc, char **argv)
 #endif
 					break;
 				case 't':
-					wait_seconds = atoi(optarg);
+					wait_seconds = strtol(optarg, &strtol_endptr, 10);
+					if (strtol_endptr != optarg + strlen(optarg))
+					{
+						write_stderr(_("%s: invalid value '%s' for -t\n"),
+								progname, optarg);
+						exit(1);
+					}
 					wait_seconds_arg = true;
 					break;
 				case 'U':
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 75f08cd792..ddbf742878 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -290,6 +290,7 @@ main(int argc, char **argv)
 	const char *dumpencoding = NULL;
 	const char *dumpsnapshot = NULL;
 	char	   *use_role = NULL;
+	char	   *strtol_endptr = NULL;
 	int			numWorkers = 1;
 	trivalue	prompt_password = TRI_DEFAULT;
 	int			compressLevel = -1;
@@ -441,7 +442,12 @@ main(int argc, char **argv)
 				break;
 
 			case 'j':			/* number of dump jobs */
-				numWorkers = atoi(optarg);
+				numWorkers = strtol(optarg, &strtol_endptr, 10);
+				if (strtol_endptr != optarg + strlen(optarg))
+				{
+					write_msg(NULL, "invalid number of jobs\n");
+					exit_nicely(1);
+				}
 				break;
 
 			case 'n':			/* include schema(s) */
@@ -507,8 +513,8 @@ main(int argc, char **argv)
 				break;
 
 			case 'Z':			/* Compression Level */
-				compressLevel = atoi(optarg);
-				if (compressLevel < 0 || compressLevel > 9)
+				compressLevel = strtol(optarg, &strtol_endptr, 10);
+				if (compressLevel < 0 || compressLevel > 9 || strtol_endptr != optarg + strlen(optarg))
 				{
 					write_msg(NULL, "compression level must be in range 0..9\n");
 					exit_nicely(1);
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 860a211a3c..e759a6e960 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -65,6 +65,7 @@ main(int argc, char **argv)
 	int			numWorkers = 1;
 	Archive    *AH;
 	char	   *inputFileSpec;
+	char	   *strtol_endptr = NULL;
 	static int	disable_triggers = 0;
 	static int	enable_row_security = 0;
 	static int	if_exists = 0;
@@ -181,7 +182,12 @@ main(int argc, char **argv)
 				break;
 
 			case 'j':			/* number of restore jobs */
-				numWorkers = atoi(optarg);
+				numWorkers = strtol(optarg, &strtol_endptr, 10);
+				if (strtol_endptr != optarg + strlen(optarg))
+				{
+					write_msg(NULL, "invalid number of jobs\n");
+					exit_nicely(1);
+				}
 				break;
 
 			case 'l':			/* Dump the TOC summary */
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index c74eb25e18..678b5a8bfa 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -60,6 +60,7 @@ parseCommandLine(int argc, char *argv[])
 	int			os_user_effective_id;
 	FILE	   *fp;
 	char	  **filename;
+	char	   *strtol_endptr = NULL;
 	time_t		run_time = time(NULL);
 
 	user_opts.transfer_mode = TRANSFER_MODE_COPY;
@@ -167,7 +168,7 @@ parseCommandLine(int argc, char *argv[])
 				 * supported on all old/new versions (added in PG 9.2).
 				 */
 			case 'p':
-				if ((old_cluster.port = atoi(optarg)) <= 0)
+				if ((old_cluster.port = strtol(optarg, &strtol_endptr, 10)) <= 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					pg_fatal("invalid old port number\n");
 					exit(1);
@@ -175,7 +176,7 @@ parseCommandLine(int argc, char *argv[])
 				break;
 
 			case 'P':
-				if ((new_cluster.port = atoi(optarg)) <= 0)
+				if ((new_cluster.port = strtol(optarg, &strtol_endptr, 10)) <= 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					pg_fatal("invalid new port number\n");
 					exit(1);
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index e37496c971..27746bcd89 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3708,6 +3708,7 @@ main(int argc, char **argv)
 	PGconn	   *con;
 	PGresult   *res;
 	char	   *env;
+	char	   *strtol_endptr;
 
 	progname = get_progname(argv[0]);
 
@@ -3766,8 +3767,8 @@ main(int argc, char **argv)
 				break;
 			case 'c':
 				benchmarking_option_set = true;
-				nclients = atoi(optarg);
-				if (nclients <= 0 || nclients > MAXCLIENTS)
+				nclients = strtol(optarg, &strtol_endptr, 10);
+				if (nclients <= 0 || nclients > MAXCLIENTS || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, "invalid number of clients: \"%s\"\n",
 							optarg);
@@ -3794,8 +3795,8 @@ main(int argc, char **argv)
 				break;
 			case 'j':			/* jobs */
 				benchmarking_option_set = true;
-				nthreads = atoi(optarg);
-				if (nthreads <= 0)
+				nthreads = strtol(optarg, &strtol_endptr, 10);
+				if (nthreads <= 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, "invalid number of threads: \"%s\"\n",
 							optarg);
@@ -3820,8 +3821,8 @@ main(int argc, char **argv)
 				break;
 			case 's':
 				scale_given = true;
-				scale = atoi(optarg);
-				if (scale <= 0)
+				scale = strtol(optarg, &strtol_endptr, 10);
+				if (scale <= 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, "invalid scaling factor: \"%s\"\n", optarg);
 					exit(1);
@@ -3834,8 +3835,8 @@ main(int argc, char **argv)
 					fprintf(stderr, "specify either a number of transactions (-t) or a duration (-T), not both\n");
 					exit(1);
 				}
-				nxacts = atoi(optarg);
-				if (nxacts <= 0)
+				nxacts = strtol(optarg, &strtol_endptr, 10);
+				if (nxacts <= 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, "invalid number of transactions: \"%s\"\n",
 							optarg);
@@ -3849,8 +3850,8 @@ main(int argc, char **argv)
 					fprintf(stderr, "specify either a number of transactions (-t) or a duration (-T), not both\n");
 					exit(1);
 				}
-				duration = atoi(optarg);
-				if (duration <= 0)
+				duration = strtol(optarg, &strtol_endptr, 10);
+				if (duration <= 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, "invalid duration: \"%s\"\n", optarg);
 					exit(1);
@@ -3916,8 +3917,8 @@ main(int argc, char **argv)
 				break;
 			case 'F':
 				initialization_option_set = true;
-				fillfactor = atoi(optarg);
-				if (fillfactor < 10 || fillfactor > 100)
+				fillfactor = strtol(optarg, &strtol_endptr, 10);
+				if (fillfactor < 10 || fillfactor > 100 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, "invalid fillfactor: \"%s\"\n", optarg);
 					exit(1);
@@ -3937,8 +3938,8 @@ main(int argc, char **argv)
 				break;
 			case 'P':
 				benchmarking_option_set = true;
-				progress = atoi(optarg);
-				if (progress <= 0)
+				progress = strtol(optarg, &strtol_endptr, 10);
+				if (progress <= 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, "invalid thread progress delay: \"%s\"\n",
 							optarg);
@@ -3999,8 +4000,8 @@ main(int argc, char **argv)
 				break;
 			case 5:
 				benchmarking_option_set = true;
-				agg_interval = atoi(optarg);
-				if (agg_interval <= 0)
+				agg_interval = strtol(optarg, &strtol_endptr, 10);
+				if (agg_interval <= 0 || strtol_endptr != optarg + strlen(optarg))
 				{
 					fprintf(stderr, "invalid number of seconds for aggregation: \"%s\"\n",
 							optarg);
-- 
2.14.1

