Convert many atoi() calls to strtonum(), adding range checks and failure

handling along the way.
Reviews by Brendan MacDonell, Jeremy Devenport, florian, doug, millert

8 files changed, 75 insertions, 30 deletions

diff --git a/bin/csh/proc.c b/bin/csh/proc.c
index b77197e1943..db11aada2df 100644
--- a/bin/csh/proc.c
+++ b/bin/csh/proc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proc.c,v 1.26 2015/02/08 06:09:50 tedu Exp $	*/
+/*	$OpenBSD: proc.c,v 1.27 2015/04/18 18:28:36 deraadt Exp $	*/
 /*	$NetBSD: proc.c,v 1.9 1995/04/29 23:21:33 mycroft Exp $	*/
 
 /*-
@@ -34,6 +34,7 @@
 #include <sys/wait.h>
 #include <errno.h>
 #include <unistd.h>
+#include <limits.h>
 #include <stdlib.h>
 #include <string.h>
 #include <stdarg.h>
@@ -931,6 +932,7 @@ void
 dokill(Char **v, struct command *t)
 {
     int signum = SIGTERM;
+    const char *errstr;
     char *name;
 
     v++;
@@ -940,8 +942,8 @@ dokill(Char **v, struct command *t)
 		if (!Isdigit(v[1][0]))
 		    stderror(ERR_NAME | ERR_BADSIG);
 
-		signum = atoi(short2str(v[1]));
-		if (signum < 0 || signum >= NSIG)
+		signum = strtonum(short2str(v[1]), 0, NSIG-1, &errstr);
+		if (errstr)
 		    stderror(ERR_NAME | ERR_BADSIG);
 		else if (signum == 0)
 		    (void) fputc('0', cshout); /* 0's symbolic name is '0' */
@@ -958,8 +960,8 @@ dokill(Char **v, struct command *t)
 	    return;
 	}
 	if (Isdigit(v[0][1])) {
-	    signum = atoi(short2str(v[0] + 1));
-	    if (signum < 0 || signum >= NSIG)
+	    signum = strtonum(short2str(v[0] + 1), 0, NSIG-1, &errstr);
+	    if (errstr)
 		stderror(ERR_NAME | ERR_BADSIG);
 	}
 	else {
@@ -1147,12 +1149,18 @@ pfind(Char *cp)
 	return (pprevious);
     }
     if (Isdigit(cp[1])) {
-	int     idx = atoi(short2str(cp + 1));
+	const char *errstr;
+	int     idx = strtonum(short2str(cp + 1), 1, INT_MAX, &errstr);
 
+	if (errstr) {
+		stderror(ERR_NAME | ERR_NOSUCHJOB);
+		return (0);
+	}
 	for (pp = proclist.p_next; pp; pp = pp->p_next)
 	    if (pp->p_index == idx && pp->p_pid == pp->p_jobid)
 		return (pp);
 	stderror(ERR_NAME | ERR_NOSUCHJOB);
+	return (0);
     }
     np = NULL;
     for (pp = proclist.p_next; pp; pp = pp->p_next)
diff --git a/bin/ksh/exec.c b/bin/ksh/exec.c
index a9ae9527cff..e12879fea33 100644
--- a/bin/ksh/exec.c
+++ b/bin/ksh/exec.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: exec.c,v 1.50 2013/06/10 21:09:27 millert Exp $	*/
+/*	$OpenBSD: exec.c,v 1.51 2015/04/18 18:28:36 deraadt Exp $	*/
 
 /*
  * execute command tree
@@ -1234,6 +1234,7 @@ do_selectargs(char **ap, bool print_menu)
 	static const char *const read_args[] = {
 		"read", "-r", "REPLY", (char *) 0
 	};
+	const char *errstr;
 	char *s;
 	int i, argct;
 
@@ -1252,8 +1253,10 @@ do_selectargs(char **ap, bool print_menu)
 			return (char *) 0;
 		s = str_val(global("REPLY"));
 		if (*s) {
-			i = atoi(s);
-			return (i >= 1 && i <= argct) ? ap[i - 1] : null;
+			i = strtonum(s, 1, argct, &errstr);
+			if (errstr)
+				return null;
+			return ap[i - 1];
 		}
 		print_menu = 1;
 	}
diff --git a/bin/ksh/jobs.c b/bin/ksh/jobs.c
index 30763443626..1369e2fcf88 100644
--- a/bin/ksh/jobs.c
+++ b/bin/ksh/jobs.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: jobs.c,v 1.40 2013/09/04 15:49:18 millert Exp $	*/
+/*	$OpenBSD: jobs.c,v 1.41 2015/04/18 18:28:36 deraadt Exp $	*/
 
 /*
  * Process and job control
@@ -1428,11 +1428,17 @@ static Job *
 j_lookup(const char *cp, int *ecodep)
 {
 	Job		*j, *last_match;
+	const char	*errstr;
 	Proc		*p;
 	int		len, job = 0;
 
 	if (digit(*cp)) {
-		job = atoi(cp);
+		job = strtonum(cp, 1, INT_MAX, &errstr);
+		if (errstr) {
+			if (ecodep)
+				*ecodep = JL_NOSUCH;
+			return (Job *) 0;
+		}
 		/* Look for last_proc->pid (what $! returns) first... */
 		for (j = job_list; j != (Job *) 0; j = j->next)
 			if (j->last_proc && j->last_proc->pid == job)
@@ -1467,7 +1473,9 @@ j_lookup(const char *cp, int *ecodep)
 
 	case '0': case '1': case '2': case '3': case '4':
 	case '5': case '6': case '7': case '8': case '9':
-		job = atoi(cp);
+		job = strtonum(cp, 1, INT_MAX, &errstr);
+		if (errstr)
+			break;
 		for (j = job_list; j != (Job *) 0; j = j->next)
 			if (j->job == job)
 				return j;
diff --git a/bin/ls/ls.c b/bin/ls/ls.c
index 075677bb23c..2621788eaee 100644
--- a/bin/ls/ls.c
+++ b/bin/ls/ls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ls.c,v 1.39 2014/03/31 20:54:37 sobrado Exp $	*/
+/*	$OpenBSD: ls.c,v 1.40 2015/04/18 18:28:36 deraadt Exp $	*/
 /*	$NetBSD: ls.c,v 1.18 1996/07/09 09:16:29 mycroft Exp $	*/
 
 /*
@@ -47,6 +47,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <limits.h>
 #include <util.h>
 
 #include "ls.h"
@@ -99,22 +100,27 @@ ls_main(int argc, char *argv[])
 	static char dot[] = ".", *dotav[] = { dot, NULL };
 	struct winsize win;
 	int ch, fts_options, notused;
-	int kflag = 0;
+	int kflag = 0, width = 0;
 	char *p;
 
 	/* Terminal defaults to -Cq, non-terminal defaults to -1. */
 	if (isatty(STDOUT_FILENO)) {
 		if ((p = getenv("COLUMNS")) != NULL)
-			termwidth = atoi(p);
-		else if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &win) == 0 &&
+			width = strtonum(p, 1, INT_MAX, NULL);
+		if (width == 0 &&
+		    ioctl(STDOUT_FILENO, TIOCGWINSZ, &win) == 0 &&
 		    win.ws_col > 0)
-			termwidth = win.ws_col;
+			width = win.ws_col;
+		if (width)
+			termwidth = width;
 		f_column = f_nonprint = 1;
 	} else {
 		f_singlecol = 1;
 		/* retrieve environment variable, in case of explicit -C */
 		if ((p = getenv("COLUMNS")) != NULL)
-			termwidth = atoi(p);
+			width = strtonum(p, 0, INT_MAX, NULL);
+		if (width)
+			termwidth = width;
 	}
 
 	/* Root is -A automatically. */
diff --git a/bin/pax/options.c b/bin/pax/options.c
index 4d6fb63c62b..be2ffb2c245 100644
--- a/bin/pax/options.c
+++ b/bin/pax/options.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: options.c,v 1.89 2015/03/15 21:53:09 guenther Exp $	*/
+/*	$OpenBSD: options.c,v 1.90 2015/04/18 18:28:37 deraadt Exp $	*/
 /*	$NetBSD: options.c,v 1.6 1996/03/26 23:54:18 mrg Exp $	*/
 
 /*-
@@ -223,6 +223,7 @@ pax_options(int argc, char **argv)
 	unsigned i;
 	unsigned int flg = 0;
 	unsigned int bflg = 0;
+	const char *errstr;
 	char *pt;
 
 	/*
@@ -462,9 +463,12 @@ pax_options(int argc, char **argv)
 			flg |= CEF;
 			if (strcmp(NONE, optarg) == 0)
 				maxflt = -1;
-			else if ((maxflt = atoi(optarg)) < 0) {
-				paxwarn(1, "Error count value must be positive");
-				pax_usage();
+			else {
+				maxflt = strtonum(optarg, 0, INT_MAX, &errstr);
+				if (errstr) {
+					paxwarn(1, "Error count value: %s", errstr);
+					pax_usage();
+				}
 			}
 			break;
 		case 'G':
@@ -1079,6 +1083,7 @@ mkpath(path)
 static void
 cpio_options(int argc, char **argv)
 {
+	const char *errstr;
 	int c;
 	unsigned i;
 	char *str;
@@ -1214,7 +1219,12 @@ cpio_options(int argc, char **argv)
 				/*
 				 * set block size in bytes
 				 */
-				wrblksz = atoi(optarg);
+				wrblksz = strtonum(optarg, 0, INT_MAX, &errstr);
+				if (errstr) {
+					paxwarn(1, "Invalid block size %s: %s",
+					    optarg, errstr);
+					pax_usage();
+				}
 				break;
 			case 'E':
 				/*
diff --git a/bin/systrace/filter.c b/bin/systrace/filter.c
index d10299605ce..73e3e8f446f 100644
--- a/bin/systrace/filter.c
+++ b/bin/systrace/filter.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: filter.c,v 1.35 2015/01/16 00:19:12 deraadt Exp $	*/
+/*	$OpenBSD: filter.c,v 1.36 2015/04/18 18:28:37 deraadt Exp $	*/
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * All rights reserved.
@@ -615,9 +615,10 @@ filter_ask(int fd, struct intercept_tlq *tls, struct filterq *fls,
 			filter_templates(emulation);
 			continue;
 		} else if (!strncasecmp(line, "template ", 9)) {
-			int count = atoi(line + 9);
+			const char *errstr;
+			int count = strtonum(line + 9, 1, INT_MAX, &errstr);
 
-			if (count == 0 ||
+			if (errstr ||
 			    filter_template(fd, policy, count) == -1) {
 				printf("Syntax error.\n");
 				continue;
diff --git a/bin/systrace/lex.l b/bin/systrace/lex.l
index 87ab4ecddfd..e23d68f0b36 100644
--- a/bin/systrace/lex.l
+++ b/bin/systrace/lex.l
@@ -1,4 +1,4 @@
-/*	$OpenBSD: lex.l,v 1.19 2015/01/16 00:19:12 deraadt Exp $	*/
+/*	$OpenBSD: lex.l,v 1.20 2015/04/18 18:28:37 deraadt Exp $	*/
 
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
@@ -103,7 +103,14 @@ as		{ return AS; }
 "<"		{ return LESSER; }
 ">"		{ return GREATER; }
 [\_\$A-Za-z][\.\(\)\/A-Za-z_\-0-9]*\$? { yylval.string = strdup(yytext); return STRING; }
-[0-9]+		{ yylval.number = atoi(yytext); return NUMBER; }
+[0-9]+		{
+		  const char *errstr;
+		  yylval.number = strtonum(yytext, 0, INT_MAX, &errstr);
+		  if (errstr) {
+			yyerror("number %s: %s", yytext, errstr);
+		  }
+		  return NUMBER;
+		}
 \"		{ BEGIN(quote);
 		  *quotestr = '\0';
 		  quoteescape = 0;
diff --git a/bin/systrace/systrace.c b/bin/systrace/systrace.c
index 2b701d9aa34..ce3b0ee90e1 100644
--- a/bin/systrace/systrace.c
+++ b/bin/systrace/systrace.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: systrace.c,v 1.62 2015/01/16 00:19:12 deraadt Exp $	*/
+/*	$OpenBSD: systrace.c,v 1.63 2015/04/18 18:28:37 deraadt Exp $	*/
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * All rights reserved.
@@ -647,6 +647,7 @@ main(int argc, char **argv)
 	char **args;
 	char *filename = NULL;
 	char *policypath = NULL;
+	const char *errstr;
 	struct timeval tv;
 	pid_t pidattach = 0;
 	int usex11 = 1;
@@ -707,7 +708,8 @@ main(int argc, char **argv)
 		case 'p':
 			if (setcredentials)
 				usage();
-			if ((pidattach = atoi(optarg)) == 0) {
+			pidattach = strtonum(optarg, 1, INT_MAX, &errstr);
+			if (errstr) {
 				warnx("bad pid: %s", optarg);
 				usage();
 			}