Add parse_uint function to improve integer overflow handling

Originally found by oss-fuzz (issue 525) in get_ansi_color using ubsan.
After a lot of analysis I'm 99% sure this isn't security relevant so
it's fine to handle this publicly.

The fix is mainly adding a function that does it right and use it
everywhere. This is harder than it seems because the strtol() family of
functions doesn't have the friendliest of interfaces.

Aside from get_ansi_color(), there were other pieces of code that used
the same (out*10+(*in-'0')) pattern, like the parse_size() and
parse_time_interval() functions, which are mostly used for settings.
Those are interesting cases, since they multiply the parsed number
(resulting in more overflows) and they write to a signed integer
parameter (which can accidentally make the uints negative without UB)

Thanks to Pascal Cuoq for enlightening me about the undefined behavior
of parse_size (and, in particular, the implementation-defined behavior
of one of the WIP versions of this commit, where something like signed
integer overflow happened, but it was legal). Also for writing
tis-interpreter, which is better than ubsan to verify these things.

1 files changed, 103 insertions, 16 deletions

diff --git a/src/core/misc.c b/src/core/misc.c
index d8437430..0f038cbb 100644
--- a/src/core/misc.c
+++ b/src/core/misc.c
@@ -750,10 +750,42 @@ int nearest_power(int num)
 	return n;
 }
 
-int parse_time_interval(const char *time, int *msecs)
+/* Parses unsigned integers from strings with decent error checking.
+ * Returns true on success, false otherwise (overflow, no valid number, etc)
+ * There's a 31 bit limit so the output can be assigned to signed positive ints */
+int parse_uint(const char *nptr, char **endptr, int base, guint *number)
+{
+	char *endptr_;
+	gulong parsed;
+
+	/* strtoul accepts whitespace and plus/minus signs, for some reason */
+	if (!i_isdigit(*nptr)) {
+		return FALSE;
+	}
+
+	errno = 0;
+	parsed = strtoul(nptr, &endptr_, base);
+
+	if (errno || endptr_ == nptr || parsed >= (1U << 31)) {
+		return FALSE;
+	}
+
+	if (endptr) {
+		*endptr = endptr_;
+	}
+
+	if (number) {
+		*number = (guint) parsed;
+	}
+
+	return TRUE;
+}
+
+static int parse_time_interval_uint(const char *time, guint *msecs)
 {
 	const char *desc;
-	int number, sign, len, ret, digits;
+	guint number;
+	int sign, len, ret, digits;
 
 	*msecs = 0;
 
@@ -769,8 +801,11 @@ int parse_time_interval(const char *time, int *msecs)
 	}
 	for (;;) {
 		if (i_isdigit(*time)) {
-			number = number*10 + (*time - '0');
-			time++;
+			char *endptr;
+			if (!parse_uint(time, &endptr, 10, &number)) {
+				return FALSE;
+			}
+			time = endptr;
 			digits = TRUE;
 			continue;
 		}
@@ -835,10 +870,11 @@ int parse_time_interval(const char *time, int *msecs)
 	return ret;
 }
 
-int parse_size(const char *size, int *bytes)
+static int parse_size_uint(const char *size, guint *bytes)
 {
 	const char *desc;
-	int number, len;
+	guint number, multiplier, limit;
+	int len;
 
 	*bytes = 0;
 
@@ -846,8 +882,11 @@ int parse_size(const char *size, int *bytes)
 	number = 0;
 	while (*size != '\0') {
 		if (i_isdigit(*size)) {
-			number = number*10 + (*size - '0');
-			size++;
+			char *endptr;
+			if (!parse_uint(size, &endptr, 10, &number)) {
+				return FALSE;
+			}
+			size = endptr;
 			continue;
 		}
 
@@ -869,14 +908,31 @@ int parse_size(const char *size, int *bytes)
 			return FALSE;
 		}
 
-		if (g_ascii_strncasecmp(desc, "gbytes", len) == 0)
-			*bytes += number * 1024*1024*1024;
-		if (g_ascii_strncasecmp(desc, "mbytes", len) == 0)
-			*bytes += number * 1024*1024;
-		if (g_ascii_strncasecmp(desc, "kbytes", len) == 0)
-			*bytes += number * 1024;
-		if (g_ascii_strncasecmp(desc, "bytes", len) == 0)
-			*bytes += number;
+		multiplier = 0;
+		limit = 0;
+
+		if (g_ascii_strncasecmp(desc, "gbytes", len) == 0) {
+			multiplier = 1U << 30;
+			limit = 2U << 0;
+		}
+		if (g_ascii_strncasecmp(desc, "mbytes", len) == 0) {
+			multiplier = 1U << 20;
+			limit = 2U << 10;
+		}
+		if (g_ascii_strncasecmp(desc, "kbytes", len) == 0) {
+			multiplier = 1U << 10;
+			limit = 2U << 20;
+		}
+		if (g_ascii_strncasecmp(desc, "bytes", len) == 0) {
+			multiplier = 1;
+			limit = 2U << 30;
+		}
+
+		if (limit && number > limit) {
+			return FALSE;
+		}
+
+		*bytes += number * multiplier;
 
 		/* skip punctuation */
 		while (*size != '\0' && i_ispunct(*size))
@@ -886,6 +942,37 @@ int parse_size(const char *size, int *bytes)
 	return TRUE;
 }
 
+int parse_size(const char *size, int *bytes)
+{
+	guint bytes_;
+	int ret;
+
+	ret = parse_size_uint(size, &bytes_);
+
+	if (bytes_ > (1U << 31)) {
+		return FALSE;
+	}
+
+	*bytes = bytes_;
+	return ret;
+}
+
+int parse_time_interval(const char *time, int *msecs)
+{
+	guint msecs_;
+	int ret;
+
+	ret = parse_time_interval_uint(time, &msecs_);
+
+	if (msecs_ > (1U << 31)) {
+		return FALSE;
+	}
+
+	*msecs = msecs_;
+	return ret;
+}
+
+
 char *ascii_strup(char *str)
 {
 	char *s;