From: Alejandro Colomar <alx@kernel.org>
Date: Fri, 1 Dec 2023 18:31:16 +0000 (+0100)
Subject: lib/, src/: Fix error handling after strto[u]l[l](3)
X-Git-Tag: 4.15.0-rc1~90
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1c464d9a2df7da655eb85f6239beb23527575f9b;p=thirdparty%2Fshadow.git

lib/, src/: Fix error handling after strto[u]l[l](3)

-  Set errno = 0 before the call.  Otherwise, it may contain anything.
-  ERANGE is not the only possible errno value of these functions.  They
   can also set it to EINVAL.
-  Any errno value after these calls is bad; just compare against 0.
-  Don't check for the return value; just errno.  This function is
   guaranteed to not modify errno on success (POSIX).
-  Check endptr == str, which may or may not set EINVAL.

Suggested-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---

diff --git a/lib/get_gid.c b/lib/get_gid.c
index a43a7d68b..c9dcd1d0c 100644
--- a/lib/get_gid.c
+++ b/lib/get_gid.c
@@ -17,10 +17,10 @@ int get_gid (const char *gidstr, gid_t *gid)
 	char *endptr;
 
 	errno = 0;
-	val = strtoll (gidstr, &endptr, 10);
+	val = strtoll(gidstr, &endptr, 10);
 	if (   ('\0' == *gidstr)
 	    || ('\0' != *endptr)
-	    || (ERANGE == errno)
+	    || (0 != errno)
 	    || (/*@+longintegral@*/val != (gid_t)val)/*@=longintegral@*/) {
 		return 0;
 	}
diff --git a/lib/get_pid.c b/lib/get_pid.c
index 5eda20279..e473731fa 100644
--- a/lib/get_pid.c
+++ b/lib/get_pid.c
@@ -20,10 +20,10 @@ int get_pid (const char *pidstr, pid_t *pid)
 	char *endptr;
 
 	errno = 0;
-	val = strtoll (pidstr, &endptr, 10);
+	val = strtoll(pidstr, &endptr, 10);
 	if (   ('\0' == *pidstr)
 	    || ('\0' != *endptr)
-	    || (ERANGE == errno)
+	    || (0 != errno)
 	    || (val < 1)
 	    || (/*@+longintegral@*/val != (pid_t)val)/*@=longintegral@*/) {
 		return 0;
@@ -46,10 +46,10 @@ int get_pidfd_from_fd(const char *pidfdstr)
 	dev_t proc_st_dev, proc_st_rdev;
 
 	errno = 0;
-	val = strtoll (pidfdstr, &endptr, 10);
+	val = strtoll(pidfdstr, &endptr, 10);
 	if (   ('\0' == *pidfdstr)
 	    || ('\0' != *endptr)
-	    || (ERANGE == errno)
+	    || (0 != errno)
 	    || (val < 0)
 	    || (/*@+longintegral@*/val != (int)val)/*@=longintegral@*/) {
 		return -1;
diff --git a/lib/get_uid.c b/lib/get_uid.c
index 797b4bb91..011b0adee 100644
--- a/lib/get_uid.c
+++ b/lib/get_uid.c
@@ -17,10 +17,10 @@ int get_uid (const char *uidstr, uid_t *uid)
 	char *endptr;
 
 	errno = 0;
-	val = strtoll (uidstr, &endptr, 10);
+	val = strtoll(uidstr, &endptr, 10);
 	if (   ('\0' == *uidstr)
 	    || ('\0' != *endptr)
-	    || (ERANGE == errno)
+	    || (0 != errno)
 	    || (/*@+longintegral@*/val != (uid_t)val)/*@=longintegral@*/) {
 		return 0;
 	}
diff --git a/lib/getgr_nam_gid.c b/lib/getgr_nam_gid.c
index 53a9ee56c..6d6259797 100644
--- a/lib/getgr_nam_gid.c
+++ b/lib/getgr_nam_gid.c
@@ -31,10 +31,10 @@ extern /*@only@*//*@null@*/struct group *getgr_nam_gid (/*@null@*/const char *gr
 	}
 
 	errno = 0;
-	gid = strtoll (grname, &endptr, 10);
+	gid = strtoll(grname, &endptr, 10);
 	if (   ('\0' != *grname)
 	    && ('\0' == *endptr)
-	    && (ERANGE != errno)
+	    && (0 == errno)
 	    && (/*@+longintegral@*/gid == (gid_t)gid)/*@=longintegral@*/) {
 		return xgetgrgid (gid);
 	}
diff --git a/lib/getlong.c b/lib/getlong.c
index 6317bf0fa..8c8083fbc 100644
--- a/lib/getlong.c
+++ b/lib/getlong.c
@@ -25,8 +25,8 @@ int getlong (const char *numstr, /*@out@*/long *result)
 	char *endptr;
 
 	errno = 0;
-	val = strtol (numstr, &endptr, 0);
-	if (('\0' == *numstr) || ('\0' != *endptr) || (ERANGE == errno)) {
+	val = strtol(numstr, &endptr, 0);
+	if (('\0' == *numstr) || ('\0' != *endptr) || (0 != errno)) {
 		return 0;
 	}
 
diff --git a/lib/getrange.c b/lib/getrange.c
index 82f2edfa4..fcef7d677 100644
--- a/lib/getrange.c
+++ b/lib/getrange.c
@@ -42,8 +42,8 @@ int getrange (const char *range,
 			return 0;
 		}
 		errno = 0;
-		n = strtoul (&range[1], &endptr, 10);
-		if (('\0' != *endptr) || (ERANGE == errno)) {
+		n = strtoul(&range[1], &endptr, 10);
+		if (('\0' != *endptr) || (0 != errno)) {
 			/* invalid */
 			return 0;
 		}
@@ -53,8 +53,8 @@ int getrange (const char *range,
 		*max = n;
 	} else {
 		errno = 0;
-		n = strtoul (range, &endptr, 10);
-		if (ERANGE == errno) {
+		n = strtoul(range, &endptr, 10);
+		if (endptr == range || 0 != errno) {
 			/* invalid */
 			return 0;
 		}
@@ -80,9 +80,9 @@ int getrange (const char *range,
 					*has_min = true;
 					*min = n;
 					errno = 0;
-					n = strtoul (endptr, &endptr, 10);
+					n = strtoul(endptr, &endptr, 10);
 					if (   ('\0' != *endptr)
-					    || (ERANGE == errno)) {
+					    || (0 != errno)) {
 						/* invalid */
 						return 0;
 					}
diff --git a/lib/gettime.c b/lib/gettime.c
index bb6e8443d..89afdbd57 100644
--- a/lib/gettime.c
+++ b/lib/gettime.c
@@ -37,9 +37,8 @@
 		return fallback;
 
 	errno = 0;
-	epoch = strtoull (source_date_epoch, &endptr, 10);
-	if ((errno == ERANGE && (epoch == ULLONG_MAX || epoch == 0))
-			|| (errno != 0 && epoch == 0)) {
+	epoch = strtoull(source_date_epoch, &endptr, 10);
+	if (errno != 0) {
 		fprintf (shadow_logfd,
 			 _("Environment variable $SOURCE_DATE_EPOCH: strtoull: %s\n"),
 			 strerror(errno));
diff --git a/lib/getulong.c b/lib/getulong.c
index 543fcd47a..03d35bf83 100644
--- a/lib/getulong.c
+++ b/lib/getulong.c
@@ -25,10 +25,10 @@ int getulong (const char *numstr, /*@out@*/unsigned long *result)
 	char *endptr;
 
 	errno = 0;
-	val = strtoul (numstr, &endptr, 0);
+	val = strtoul(numstr, &endptr, 0);
 	if (    ('\0' == *numstr)
 	     || ('\0' != *endptr)
-	     || (ERANGE == errno)
+	     || (0 != errno)
 	   ) {
 		return 0;
 	}
diff --git a/lib/limits.c b/lib/limits.c
index 1da228ca6..1aa4dc296 100644
--- a/lib/limits.c
+++ b/lib/limits.c
@@ -62,14 +62,12 @@ static int setrlimit_value (unsigned int resource,
 		 * Also, we are limited to base 10 here (hex numbers will not
 		 * work with the limit string parser as is anyway)
 		 */
+		errno = 0;
 		l = strtol(value, &endptr, 10);
 
-		if (value == endptr) {
-			/* No argument at all. No-op.
-			 * FIXME: We could instead throw an error, though.
-			 */
-			return 0;
-		}
+		if (value == endptr || errno != 0)
+			return 0;  // FIXME: We could instead throw an error, though.
+
 		if (__builtin_mul_overflow(l, multiplier, &limit)) {
 			/* FIXME: Again, silent error handling...
 			 * Wouldn't screaming make more sense?
diff --git a/lib/prefix_flag.c b/lib/prefix_flag.c
index 7c2afe626..8360a4ab4 100644
--- a/lib/prefix_flag.c
+++ b/lib/prefix_flag.c
@@ -352,7 +352,7 @@ extern struct group *prefix_getgr_nam_gid(const char *grname)
 	gid = strtoll(grname, &endptr, 10);
 	if (   ('\0' != *grname)
 	    && ('\0' == *endptr)
-	    && (ERANGE != errno)
+	    && (0 == errno)
 	    && (gid == (gid_t)gid))
 	{
 		return prefix_getgrgid(gid);
diff --git a/src/check_subid_range.c b/src/check_subid_range.c
index 38703b603..160df728f 100644
--- a/src/check_subid_range.c
+++ b/src/check_subid_range.c
@@ -34,11 +34,12 @@ int main(int argc, char **argv)
 
 	owner = argv[1];
 	check_uids = argv[2][0] == 'u';
+	errno = 0;
 	start = strtoul(argv[3], NULL, 10);
-	if (start == ULONG_MAX && errno == ERANGE)
+	if (errno != 0)
 		exit(1);
 	count = strtoul(argv[4], NULL, 10);
-	if (count == ULONG_MAX && errno == ERANGE)
+	if (errno != 0)
 		exit(1);
 	if (check_uids) {
 		if (have_sub_uids(owner, start, count))
diff --git a/src/usermod.c b/src/usermod.c
index 06fe931cb..4fe52c8ac 100644
--- a/src/usermod.c
+++ b/src/usermod.c
@@ -318,13 +318,13 @@ static struct ulong_range getulong_range(const char *str)
 
 	errno = 0;
 	first = strtoll(str, &pos, 10);
-	if (('\0' == *str) || ('-' != *pos ) || (ERANGE == errno) ||
+	if (('\0' == *str) || ('-' != *pos ) || (0 != errno) ||
 	    (first != (unsigned long)first))
 		goto out;
 
 	errno = 0;
 	last = strtoll(pos + 1, &pos, 10);
-	if (('\0' != *pos ) || (ERANGE == errno) ||
+	if (('\0' != *pos ) || (0 != errno) ||
 	    (last != (unsigned long)last))
 		goto out;