Always use our own strtonum and implement sudo_strtoid in terms of it.

config.h.in

@@ -742,9 +742,6 @@
 /* Define to 1 if you have the `strsignal' function. */
 #undef HAVE_STRSIGNAL
 
-/* Define to 1 if you have the `strtonum' function. */
-#undef HAVE_STRTONUM
-
 /* Define to 1 if `d_namlen' is a member of `struct dirent'. */
 #undef HAVE_STRUCT_DIRENT_D_NAMLEN
 

configure

@@ -21053,30 +21053,6 @@ else
     done
 
 fi
-# We wrap OpenBSD's strtonum() to get translatable error strings.
-for ac_func in strtonum
-do :
-  ac_fn_c_check_func "$LINENO" "strtonum" "ac_cv_func_strtonum"
-if test "x$ac_cv_func_strtonum" = xyes; then :
-  cat >>confdefs.h <<_ACEOF
-#define HAVE_STRTONUM 1
-_ACEOF
-
-fi
-done
-
-case " $LIBOBJS " in
-  *" strtonum.$ac_objext "* ) ;;
-  *) LIBOBJS="$LIBOBJS strtonum.$ac_objext"
- ;;
-esac
-
-
-    for _sym in sudo_strtonum; do
-	COMPAT_EXP="${COMPAT_EXP}${_sym}
-"
-    done
-
 ac_fn_c_check_member "$LINENO" "struct tm" "tm_gmtoff" "ac_cv_member_struct_tm_tm_gmtoff" "
 $ac_includes_default
 #include <errno.h>

configure.ac

@@ -2800,10 +2800,6 @@ else
     # Missing or non-compliant v?snprintf(), assume missing/bad v?asprintf()
     SUDO_APPEND_COMPAT_EXP(sudo_snprintf sudo_vsnprintf sudo_asprintf sudo_vasprintf)
 fi
-# We wrap OpenBSD's strtonum() to get translatable error strings.
-AC_CHECK_FUNCS([strtonum])
-AC_LIBOBJ(strtonum)
-SUDO_APPEND_COMPAT_EXP(sudo_strtonum)
 AC_CHECK_MEMBERS([struct tm.tm_gmtoff], [], [], [
 AC_INCLUDES_DEFAULT
 #include <errno.h>

include/sudo_compat.h

@@ -379,7 +379,7 @@ int getdomainname(char *, size_t);
 # endif
 #endif /* __hpux && !__LP64__ */
 
-/* We wrap OpenBSD's strtonum() to get translatable error strings. */
+/* We use our own strtonum() to get translatable error strings. */
 __dso_public long long sudo_strtonum(const char *, long long, long long, const char **);
 #undef strtonum
 #define strtonum(_a, _b, _c, _d) sudo_strtonum((_a), (_b), (_c), (_d))

lib/util/Makefile.in

@@ -117,14 +117,15 @@ SHELL = @SHELL@
 LTOBJS = @DIGEST@ event.lo fatal.lo key_val.lo gethostname.lo gettime.lo \
 	 getgrouplist.lo gidlist.lo lbuf.lo locking.lo parseln.lo progname.lo \
 	 secure_path.lo setgroups.lo strsplit.lo strtobool.lo strtoid.lo \
-	 strtomode.lo sudo_conf.lo sudo_debug.lo sudo_dso.lo term.lo \
+	 strtomode.lo strtonum.lo sudo_conf.lo sudo_debug.lo sudo_dso.lo \
-	 ttyname_dev.lo ttysize.lo @COMMON_OBJS@ @LTLIBOBJS@
+	 term.lo ttyname_dev.lo ttysize.lo @COMMON_OBJS@ @LTLIBOBJS@
 
 IOBJS = $(LTOBJS:.lo=.i)
 
 POBJS = $(IOBJS:.i=.plog)
 
-ATOFOO_TEST_OBJS = atofoo_test.lo strtobool.lo strtoid.lo strtomode.lo
+ATOFOO_TEST_OBJS = atofoo_test.lo strtobool.lo strtoid.lo strtomode.lo \
+		   strtonum.lo
 
 MKTEMP_TEST_OBJS = mktemp_test.lo mktemp.lo
 

lib/util/strtoid.c

@@ -48,6 +48,9 @@
 #include "sudo_debug.h"
 #include "sudo_util.h"
 
+/* strtoid.c (not exported) */
+long long sudo_strtonumx(const char *str, long long minval, long long maxval, char **ep, const char **errstrp);
+
 /*
  * Make sure that the ID ends with a valid separator char.
  */
@@ -55,7 +58,6 @@ static bool
 valid_separator(const char *p, const char *ep, const char *sep)
 {
     bool valid = false;
-    debug_decl(valid_separator, SUDO_DEBUG_UTIL)
 
     if (ep != p) {
 	/* check for valid separator (including '\0') */
@@ -66,7 +68,7 @@ valid_separator(const char *p, const char *ep, const char *sep)
 		valid = true;
 	} while (*sep++ != '\0');
     }
-    debug_return_bool(valid);
+    return valid;
 }
 
 /*
@@ -76,109 +78,29 @@ valid_separator(const char *p, const char *ep, const char *sep)
  * On success, returns the parsed ID and clears errstr.
  * On error, returns 0 and sets errstr.
  */
-#if SIZEOF_ID_T == SIZEOF_LONG_LONG
 id_t
-sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr)
+sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstrp)
 {
+    const char *errstr;
     char *ep;
-    id_t ret = 0;
+    id_t ret;
-    long long llval;
     debug_decl(sudo_strtoid, SUDO_DEBUG_UTIL)
 
-    /* skip leading space so we can pick up the sign, if any */
+    ret = sudo_strtonumx(p, INT_MIN, UINT_MAX, &ep, &errstr);
-    while (isspace((unsigned char)*p))
+    if (errstr == NULL) {
-	p++;
+	/*
-
+	 * Disallow id -1 (UINT_MAX), which means "no change"
-    /* While id_t may be 64-bit signed, uid_t and gid_t are 32-bit unsigned. */
+	 * and check for a valid separator (if specified).
-    errno = 0;
+	 */
-    llval = strtoll(p, &ep, 10);
+	if (ret == (id_t)-1 || ret == (id_t)UINT_MAX || !valid_separator(p, ep, sep)) {
-    if ((errno == ERANGE && llval == LLONG_MAX) || llval > (id_t)UINT_MAX) {
+	    errstr = N_("invalid value");
-	errno = ERANGE;
-	if (errstr != NULL)
-	    *errstr = N_("value too large");
-	goto done;
-    }
-    if ((errno == ERANGE && llval == LLONG_MIN) || llval < INT_MIN) {
-	errno = ERANGE;
-	if (errstr != NULL)
-	    *errstr = N_("value too small");
-	goto done;
-    }
-
-    /* Disallow id -1, which means "no change". */
-    if (!valid_separator(p, ep, sep) || llval == -1 || llval == (id_t)UINT_MAX) {
-	if (errstr != NULL)
-	    *errstr = N_("invalid value");
 	    errno = EINVAL;
-	goto done;
+	    ret = 0;
 	}
-    ret = (id_t)llval;
+    }
-    if (errstr != NULL)
+    if (errstrp != NULL)
-	*errstr = NULL;
+	*errstrp = errstr;
     if (endp != NULL)
 	*endp = ep;
-done:
     debug_return_id_t(ret);
 }
-#else
-id_t
-sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr)
-{
-    char *ep;
-    id_t ret = 0;
-    debug_decl(sudo_strtoid, SUDO_DEBUG_UTIL)
-
-    /* skip leading space so we can pick up the sign, if any */
-    while (isspace((unsigned char)*p))
-	p++;
-
-    errno = 0;
-    if (*p == '-') {
-	long lval = strtol(p, &ep, 10);
-	if ((errno == ERANGE && lval == LONG_MAX) || lval > INT_MAX) {
-	    errno = ERANGE;
-	    if (errstr != NULL)
-		*errstr = N_("value too large");
-	    goto done;
-	}
-	if ((errno == ERANGE && lval == LONG_MIN) || lval < INT_MIN) {
-	    errno = ERANGE;
-	    if (errstr != NULL)
-		*errstr = N_("value too small");
-	    goto done;
-	}
-
-	/* Disallow id -1, which means "no change". */
-	if (!valid_separator(p, ep, sep) || lval == -1) {
-	    if (errstr != NULL)
-		*errstr = N_("invalid value");
-	    errno = EINVAL;
-	    goto done;
-	}
-	ret = (id_t)lval;
-    } else {
-	unsigned long ulval = strtoul(p, &ep, 10);
-	if ((errno == ERANGE && ulval == ULONG_MAX) || ulval > UINT_MAX) {
-	    errno = ERANGE;
-	    if (errstr != NULL)
-		*errstr = N_("value too large");
-	    goto done;
-	}
-
-	/* Disallow id -1, which means "no change". */
-	if (!valid_separator(p, ep, sep) || ulval == UINT_MAX) {
-	    if (errstr != NULL)
-		*errstr = N_("invalid value");
-	    errno = EINVAL;
-	    goto done;
-	}
-	ret = (id_t)ulval;
-    }
-    if (errstr != NULL)
-	*errstr = NULL;
-    if (endp != NULL)
-	*endp = ep;
-done:
-    debug_return_id_t(ret);
-}
-#endif /* SIZEOF_ID_T == 8 */

lib/util/strtonum.c

@@ -1,7 +1,7 @@
 /*
  * SPDX-License-Identifier: ISC
  *
- * Copyright (c) 2013-2014 Todd C. Miller <Todd.Miller@sudo.ws>
+ * Copyright (c) 2013-2015, 2019 Todd C. Miller <Todd.Miller@sudo.ws>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -42,40 +42,8 @@
 
 #include "sudo_compat.h"
 
-#ifdef HAVE_STRTONUM
-
-/*
- * The OpenBSD strtonum error string too short to be translated sensibly.
- * This wrapper just changes errstr as follows:
- *  invalid -> invalid value
- *  too large -> value too large
- *  too small -> value too small
- */
-long long
-sudo_strtonum(const char *str, long long minval, long long maxval,
-    const char **errstrp)
-{
-    long long retval;
-    const char *errstr;
-
-# undef strtonum
-    retval = strtonum(str, minval, maxval, &errstr);
-    if (errstr != NULL) {
-	if (errno == EINVAL) {
-	    errstr = N_("invalid value");
-	} else if (errno == ERANGE) {
-	    errstr = strcmp(errstr, "too large") == 0 ?
-		N_("value too large") : N_("value too small");
-	}
-    }
-    if (errstrp != NULL)
-	*errstrp = errstr;
-    return retval;
-}
-
-#else
-
 enum strtonum_err {
+    STN_INITIAL,
     STN_VALID,
     STN_INVALID,
     STN_TOOSMALL,
@@ -84,16 +52,18 @@ enum strtonum_err {
 
 /*
  * Convert a string to a number in the range [minval, maxval]
+ * Unlike strtonum(), this returns the first non-digit in endp (if not NULL).
  */
 long long
-sudo_strtonum(const char *str, long long minval, long long maxval,
+sudo_strtonumx(const char *str, long long minval, long long maxval, char **endp,
     const char **errstrp)
 {
-    const unsigned char *ustr = (const unsigned char *)str;
+    enum strtonum_err errval = STN_INITIAL;
-    enum strtonum_err errval = STN_VALID;
     long long lastval, result = 0;
-    unsigned char dig, sign;
+    const char *cp = str;
+    unsigned char ch;
     int remainder;
+    char sign;
 
     if (minval > maxval) {
 	errval = STN_INVALID;
@@ -101,16 +71,16 @@ sudo_strtonum(const char *str, long long minval, long long maxval,
     }
 
     /* Trim leading space and check sign, if any. */
-    while (isspace(*ustr)) {
+    do {
-	ustr++;
+	ch = *cp++;
-    }
+    } while (isspace(ch));
-    switch (*ustr) {
+    switch (ch) {
     case '-':
 	sign = '-';
-	ustr++;
+	ch = *cp++;
 	break;
     case '+':
-	ustr++;
+	ch = *cp++;
 	/* FALLTHROUGH */
     default:
 	sign = '+';
@@ -133,18 +103,17 @@ sudo_strtonum(const char *str, long long minval, long long maxval,
 	    lastval += 1;
 	    remainder += 10;
 	}
-	while ((dig = *ustr++) != '\0') {
+	for (;; ch = *cp++) {
-	    if (!isdigit(dig)) {
+	    if (!isdigit(ch))
-		errval = STN_INVALID;
 		break;
-	    }
+	    ch -= '0';
-	    dig -= '0';
+	    if (result < lastval || (result == lastval && ch > remainder)) {
-	    if (result < lastval || (result == lastval && dig > remainder)) {
 		errval = STN_TOOSMALL;
 		break;
 	    } else {
+		errval = STN_VALID;
 		result *= 10;
-		result -= dig;
+		result -= ch;
 	    }
 	}
 	if (result > maxval)
@@ -152,18 +121,17 @@ sudo_strtonum(const char *str, long long minval, long long maxval,
     } else {
 	lastval = maxval / 10;
 	remainder = maxval % 10;
-	while ((dig = *ustr++) != '\0') {
+	for (;; ch = *cp++) {
-	    if (!isdigit(dig)) {
+	    if (!isdigit(ch))
-		errval = STN_INVALID;
 		break;
-	    }
+	    ch -= '0';
-	    dig -= '0';
+	    if (result > lastval || (result == lastval && ch > remainder)) {
-	    if (result > lastval || (result == lastval && dig > remainder)) {
 		errval = STN_TOOBIG;
 		break;
 	    } else {
+		errval = STN_VALID;
 		result *= 10;
-		result += dig;
+		result += ch;
 	    }
 	}
 	if (result < minval)
@@ -172,6 +140,7 @@ sudo_strtonum(const char *str, long long minval, long long maxval,
 
 done:
     switch (errval) {
+    case STN_INITIAL:
     case STN_VALID:
 	if (errstrp != NULL)
 	    *errstrp = NULL;
@@ -183,18 +152,48 @@ done:
 	    *errstrp = N_("invalid value");
 	break;
     case STN_TOOSMALL:
+	/* Skip remaining digits. */
+	while (isdigit(ch))
+	    ch = *cp++;
 	result = 0;
 	errno = ERANGE;
 	if (errstrp != NULL)
 	    *errstrp = N_("value too small");
 	break;
     case STN_TOOBIG:
+	/* Skip remaining digits. */
+	while (isdigit(ch))
+	    ch = *cp++;
 	result = 0;
 	errno = ERANGE;
 	if (errstrp != NULL)
 	    *errstrp = N_("value too large");
 	break;
     }
+    if (endp != NULL)
+	*endp = (char *)(errval == STN_INITIAL ? str : cp - 1);
     return result;
 }
-#endif /* HAVE_STRTONUM */
+
+/*
+ * Convert a string to a number in the range [minval, maxval]
+ */
+long long
+sudo_strtonum(const char *str, long long minval, long long maxval,
+    const char **errstrp)
+{
+    const char *errstr;
+    char *ep;
+    long long ret;
+
+    ret = sudo_strtonumx(str, minval, maxval, &ep, &errstr);
+    /* Check for empty string and terminating NUL. */
+    if (str == ep || *ep != '\0') {
+	errno = EINVAL;
+	errstr = N_("invalid value");
+	ret = 0;
+    }
+    if (errstrp != NULL)
+	*errstrp = errstr;
+    return ret;
+}

lib/util/util.exp.in

@@ -99,6 +99,7 @@ sudo_strsplit_v1
 sudo_strtobool_v1
 sudo_strtoid_v1
 sudo_strtomode_v1
+sudo_strtonum
 sudo_term_cbreak_v1
 sudo_term_copy_v1
 sudo_term_eof

mkdep.pl

@@ -116,7 +116,7 @@ sub mkdep {
     # XXX - fill in AUTH_OBJS from contents of the auth dir instead
     $makefile =~ s:\@AUTH_OBJS\@:afs.lo aix_auth.lo bsdauth.lo dce.lo fwtk.lo getspwuid.lo kerb5.lo pam.lo passwd.lo rfc1938.lo secureware.lo securid5.lo sia.lo:;
     $makefile =~ s:\@DIGEST\@:digest.lo digest_openssl.lo digest_gcrypt.lo:;
-    $makefile =~ s:\@LTLIBOBJS\@:arc4random.lo arc4random_uniform.lo closefrom.lo fnmatch.lo getaddrinfo.lo getcwd.lo getentropy.lo getgrouplist.lo getdelim.lo getopt_long.lo glob.lo inet_ntop_lo inet_pton.lo isblank.lo memrchr.lo memset_s.lo mksiglist.lo mksigname.lo mktemp.lo nanosleep.lo pw_dup.lo reallocarray.lo sha2.lo sig2str.lo siglist.lo signame.lo snprintf.lo str2sig.lo strlcat.lo strlcpy.lo strndup.lo strnlen.lo strsignal.lo strtonum.lo utimens.lo vsyslog.lo pipe2.lo:;
+    $makefile =~ s:\@LTLIBOBJS\@:arc4random.lo arc4random_uniform.lo closefrom.lo fnmatch.lo getaddrinfo.lo getcwd.lo getentropy.lo getgrouplist.lo getdelim.lo getopt_long.lo glob.lo inet_ntop_lo inet_pton.lo isblank.lo memrchr.lo memset_s.lo mksiglist.lo mksigname.lo mktemp.lo nanosleep.lo pw_dup.lo reallocarray.lo sha2.lo sig2str.lo siglist.lo signame.lo snprintf.lo str2sig.lo strlcat.lo strlcpy.lo strndup.lo strnlen.lo strsignal.lo utimens.lo vsyslog.lo pipe2.lo:;
 
     # Parse OBJS lines
     my %objs;