Reject negative numbers in strtoul(3)

lib/Makefile.am

@@ -30,6 +30,8 @@ libshadow_la_SOURCES = \
 	agetpass.h \
 	alloc.c \
 	alloc.h \
+	atoi/strtou_noneg.c \
+	atoi/strtou_noneg.h \
 	attr.h \
 	audit_help.c \
 	basename.c \

lib/atoi/strtou_noneg.c

@@ -0,0 +1,13 @@
+// SPDX-FileCopyrightText: 2023, Alejandro Colomar <alx@kernel.org>
+// SPDX-License-Identifier: BSD-3-Clause
+
+
+#include <config.h>
+
+#include "atoi/strtou_noneg.h"
+
+
+extern inline unsigned long strtoul_noneg(const char *s,
+    char **restrict endp, int base);
+extern inline unsigned long long strtoull_noneg(const char *s,
+    char **restrict endp, int base);

lib/atoi/strtou_noneg.h

@@ -0,0 +1,47 @@
+// SPDX-FileCopyrightText: 2023, Alejandro Colomar <alx@kernel.org>
+// SPDX-License-Identifier: BSD-3-Clause
+
+
+#ifndef SHADOW_INCLUDE_LIB_ATOI_STRTOU_NONEG_H_
+#define SHADOW_INCLUDE_LIB_ATOI_STRTOU_NONEG_H_
+
+
+#include <config.h>
+
+#include <errno.h>
+#include <stdlib.h>
+
+#include "attr.h"
+
+
+ATTR_STRING(1) ATTR_ACCESS(write_only, 2)
+inline unsigned long strtoul_noneg(const char *s,
+    char **restrict endp, int base);
+ATTR_STRING(1) ATTR_ACCESS(write_only, 2)
+inline unsigned long long strtoull_noneg(const char *s,
+    char **restrict endp, int base);
+
+
+inline unsigned long
+strtoul_noneg(const char *s, char **restrict endp, int base)
+{
+	if (strtol(s, endp, base) < 0) {
+		errno = ERANGE;
+		return 0;
+	}
+	return strtoul(s, endp, base);
+}
+
+
+inline unsigned long long
+strtoull_noneg(const char *s, char **restrict endp, int base)
+{
+	if (strtol(s, endp, base) < 0) {
+		errno = ERANGE;
+		return 0;
+	}
+	return strtoull(s, endp, base);
+}
+
+
+#endif  // include guard

lib/getrange.c

@@ -12,6 +12,7 @@
 #include <ctype.h>
 #include <stdlib.h>
 
+#include "atoi/strtou_noneg.h"
 #include "defines.h"
 #include "prototypes.h"
 
@@ -40,7 +41,7 @@ getrange(const char *range,
 			return -1;
 
 		errno = 0;
-		n = strtoul(&range[1], &endptr, 10);
+		n = strtoul_noneg(&range[1], &endptr, 10);
 		if (('\0' != *endptr) || (0 != errno))
 			return -1;
 
@@ -50,7 +51,7 @@ getrange(const char *range,
 		*max = n;
 	} else {
 		errno = 0;
-		n = strtoul(range, &endptr, 10);
+		n = strtoul_noneg(range, &endptr, 10);
 		if (endptr == range || 0 != errno)
 			return -1;
 
@@ -75,7 +76,7 @@ getrange(const char *range,
 				*has_min = true;
 				*min = n;
 				errno = 0;
-				n = strtoul(endptr, &endptr, 10);
+				n = strtoul_noneg(endptr, &endptr, 10);
 				if ('\0' != *endptr || 0 != errno)
 					return -1;
 

lib/gettime.c

@@ -11,6 +11,8 @@
 #include <errno.h>
 #include <limits.h>
 #include <stdio.h>
+
+#include "atoi/strtou_noneg.h"
 #include "defines.h"
 #include "prototypes.h"
 #include "shadowlog.h"
@@ -37,7 +39,7 @@
 		return fallback;
 
 	errno = 0;
-	epoch = strtoull(source_date_epoch, &endptr, 10);
+	epoch = strtoull_noneg(source_date_epoch, &endptr, 10);
 	if (errno != 0) {
 		fprintf (shadow_logfd,
 			 _("Environment variable $SOURCE_DATE_EPOCH: strtoull: %s\n"),

lib/getulong.c

@@ -12,6 +12,7 @@
 #include <stdlib.h>
 #include <errno.h>
 
+#include "atoi/strtou_noneg.h"
 #include "prototypes.h"
 
 
@@ -27,7 +28,7 @@ getulong(const char *restrict numstr, unsigned long *restrict result)
 	unsigned long  val;
 
 	errno = 0;
-	val = strtoul(numstr, &endptr, 0);
+	val = strtoul_noneg(numstr, &endptr, 0);
 	if (('\0' == *numstr) || ('\0' != *endptr) || (0 != errno))
 		return -1;
 

src/check_subid_range.c

@@ -12,6 +12,8 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+
+#include "atoi/strtou_noneg.h"
 #include "defines.h"
 #include "prototypes.h"
 #include "subordinateio.h"
@@ -35,10 +37,10 @@ int main(int argc, char **argv)
 	owner = argv[1];
 	check_uids = argv[2][0] == 'u';
 	errno = 0;
-	start = strtoul(argv[3], NULL, 10);
+	start = strtoul_noneg(argv[3], NULL, 10);
 	if (errno != 0)
 		exit(1);
-	count = strtoul(argv[4], NULL, 10);
+	count = strtoul_noneg(argv[4], NULL, 10);
 	if (errno != 0)
 		exit(1);
 	if (check_uids) {

tests/unit/Makefile.am

@@ -5,6 +5,7 @@ TESTS = $(check_PROGRAMS)
 
 check_PROGRAMS = \
     test_adds \
+    test_atoi_strtou_noneg \
     test_chkname \
     test_sprintf \
     test_strncpy \
@@ -32,6 +33,19 @@ test_adds_LDADD = \
     $(CMOCKA_LIBS) \
     $(NULL)
 
+test_atoi_strtou_noneg_SOURCES = \
+    ../../lib/atoi/strtou_noneg.c \
+    test_atoi_strtou_noneg.c \
+    $(NULL)
+test_atoi_strtou_noneg_CFLAGS = \
+    $(AM_FLAGS) \
+    $(NULL)
+test_atoi_strtou_noneg_LDFLAGS = \
+    $(NULL)
+test_atoi_strtou_noneg_LDADD = \
+    $(CMOCKA_LIBS) \
+    $(NULL)
+
 test_chkname_SOURCES = \
     ../../lib/chkname.c \
     test_chkname.c \

tests/unit/test_atoi_strtou_noneg.c

@@ -0,0 +1,76 @@
+// SPDX-FileCopyrightText: 2023-2024, Alejandro Colomar <alx@kernel.org>
+// SPDX-License-Identifier: BSD-3-Clause
+
+
+#include <errno.h>
+#include <limits.h>
+#include <stddef.h>
+#include <stdlib.h>
+
+#include <stdarg.h>  // Required by <cmocka.h>
+#include <stddef.h>  // Required by <cmocka.h>
+#include <setjmp.h>  // Required by <cmocka.h>
+#include <stdint.h>  // Required by <cmocka.h>
+#include <cmocka.h>
+
+#include "atoi/strtou_noneg.h"
+
+
+static void test_strtoul_noneg(void **state);
+static void test_strtoull_noneg(void **state);
+
+
+int
+main(void)
+{
+    const struct CMUnitTest  tests[] = {
+        cmocka_unit_test(test_strtoul_noneg),
+        cmocka_unit_test(test_strtoull_noneg),
+    };
+
+    return cmocka_run_group_tests(tests, NULL, NULL);
+}
+
+
+static void
+test_strtoul_noneg(void **state)
+{
+	errno = 0;
+	assert_true(strtoul_noneg("42", NULL, 0) == 42);
+	assert_true(errno == 0);
+
+	assert_true(strtoul_noneg("-1", NULL, 0) == 0);
+	assert_true(errno == ERANGE);
+	errno = 0;
+	assert_true(strtoul_noneg("-3", NULL, 0) == 0);
+	assert_true(errno == ERANGE);
+	errno = 0;
+	assert_true(strtoul_noneg("-0xFFFFFFFFFFFFFFFF", NULL, 0) == 0);
+	assert_true(errno == ERANGE);
+
+	errno = 0;
+	assert_true(strtoul_noneg("-0x10000000000000000", NULL, 0) == 0);
+	assert_true(errno == ERANGE);
+}
+
+
+static void
+test_strtoull_noneg(void **state)
+{
+	errno = 0;
+	assert_true(strtoull_noneg("42", NULL, 0) == 42);
+	assert_true(errno == 0);
+
+	assert_true(strtoull_noneg("-1", NULL, 0) == 0);
+	assert_true(errno == ERANGE);
+	errno = 0;
+	assert_true(strtoull_noneg("-3", NULL, 0) == 0);
+	assert_true(errno == ERANGE);
+	errno = 0;
+	assert_true(strtoull_noneg("-0xFFFFFFFFFFFFFFFF", NULL, 0) == 0);
+	assert_true(errno == ERANGE);
+
+	errno = 0;
+	assert_true(strtoull_noneg("-0x10000000000000000", NULL, 0) == 0);
+	assert_true(errno == ERANGE);
+}