From=20da448337bd050518323b45fd5d2929d04b552802 Mon Sep 17 00:00:00 2001
From: Pierre Ducroquet <pierre.ducroquet@people-doc.com>
Date: Tue, 18 Apr 2017 09:40:40 +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                |  8 +++++++-
 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              | 34 ++++++++++++++++++----------------
 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 40ec0e17dc..34884fce4f 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;
@@ -2063,6 +2063,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'},
@@ -2203,8 +2204,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);
@@ -2242,8 +2243,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 1a9fe81be1..f470406678 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -479,6 +479,7 @@ main(int argc, char **argv)
 	int			c;
 	int			option_index;
 	char	   *db_name;
+    char       *strtol_endptr = NULL;
 
 	progname = get_progname(argv[0]);
 	set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_basebackup"));
@@ -513,7 +514,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);
@@ -531,8 +532,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);
@@ -549,8 +550,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 6b081bd737..cd69a6fba6 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;
 
 	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 c63819b88b..65191a5319 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2177,6 +2177,7 @@ main(int argc, char **argv)
 	};
 
 	char	   *env_wait;
+    char       *strtol_endptr;
 	int			option_index;
 	int			c;
 	pgpid_t		killproc = 0;
@@ -2307,7 +2308,12 @@ 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 e9b5c8a448..97a4ac5f44 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -296,6 +296,7 @@ main(int argc, char **argv)
 	int			plainText = 0;
 	ArchiveFormat archiveFormat = archUnknown;
 	ArchiveMode archiveMode;
+	char       *strtol_endptr = NULL;
 
 	static DumpOptions dopt;
 
@@ -438,7 +439,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) */
@@ -504,8 +510,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 ddd79429b4..dac5f7b2de 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -73,6 +73,7 @@ main(int argc, char **argv)
 	static int	use_setsessauth = 0;
 	static int	no_security_labels = 0;
 	static int	strict_names = 0;
+	char       *strtol_endptr = NULL;
 
 	struct option cmdopts[] = {
 		{"clean", 0, NULL, 'c'},
@@ -177,7 +178,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 5007ce53cf..b4acbdc727 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;
 	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 ae3624721e..e34786747b 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3680,6 +3680,8 @@ main(int argc, char **argv)
 	PGresult   *res;
 	char	   *env;
 
+    char       *strtol_endptr;
+
 	progname = get_progname(argv[0]);
 
 	if (argc > 1)
@@ -3737,8 +3739,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);
@@ -3765,8 +3767,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);
@@ -3791,8 +3793,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);
@@ -3805,8 +3807,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);
@@ -3820,8 +3822,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);
@@ -3887,8 +3889,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);
@@ -3913,8 +3915,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);
@@ -3975,8 +3977,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.11.0

