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.

4 files changed, 130 insertions, 27 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;
diff --git a/src/core/misc.h b/src/core/misc.h
index 00637da0..375744db 100644
--- a/src/core/misc.h
+++ b/src/core/misc.h
@@ -71,6 +71,7 @@ int expand_escape(const char **data);
 int nearest_power(int num);
 
 /* Returns TRUE / FALSE */
+int parse_uint(const char *nptr, char **endptr, int base, guint *number);
 int parse_time_interval(const char *time, int *msecs);
 int parse_size(const char *size, int *bytes);
 
diff --git a/src/core/special-vars.c b/src/core/special-vars.c
index 6ca080fc..aaf8da8f 100644
--- a/src/core/special-vars.c
+++ b/src/core/special-vars.c
@@ -275,6 +275,8 @@ static char *get_special_value(char **cmd, SERVER_REC *server, void *item,
 static int get_alignment_args(char **data, int *align, int *flags, char *pad)
 {
 	char *str;
+	char *endptr;
+	guint align_;
 
 	*align = 0;
 	*flags = ALIGN_CUT|ALIGN_PAD;
@@ -295,10 +297,11 @@ static int get_alignment_args(char **data, int *align, int *flags, char *pad)
 		return FALSE; /* expecting number */
 
 	/* get the alignment size */
-	while (i_isdigit(*str)) {
-		*align = (*align) * 10 + (*str-'0');
-		str++;
+	if (!parse_uint(str, &endptr, 10, &align_)) {
+		return FALSE;
 	}
+	str = endptr;
+	*align = align_;
 
 	/* get the pad character */
 	while (*str != '\0' && *str != ']') {
diff --git a/src/fe-common/core/formats.c b/src/fe-common/core/formats.c
index 17c13a97..005e6fb7 100644
--- a/src/fe-common/core/formats.c
+++ b/src/fe-common/core/formats.c
@@ -33,6 +33,7 @@
 #include "themes.h"
 #include "recode.h"
 #include "utf8.h"
+#include "misc.h"
 
 static const char *format_backs = "04261537";
 static const char *format_fores = "kbgcrmyw";
@@ -870,8 +871,9 @@ static const char *get_ansi_color(THEME_REC *theme, const char *str,
 {
 	static char ansitab[8] = { 0, 4, 2, 6, 1, 5, 3, 7 };
 	const char *start;
-	int fg, bg, flags, num, i;
-	unsigned int num2;
+	char *endptr;
+	int fg, bg, flags, i;
+	guint num, num2;
 
 	if (*str != '[')
 		return str;
@@ -886,8 +888,10 @@ static const char *get_ansi_color(THEME_REC *theme, const char *str,
 		if (*str == '\0') return start;
 
 		if (i_isdigit(*str)) {
-			num = num*10 + (*str-'0');
-			continue;
+			if (!parse_uint(str, &endptr, 10, &num)) {
+				return start;
+			}
+			str = endptr;
 		}
 
 		if (*str != ';' && *str != 'm')
@@ -958,8 +962,12 @@ static const char *get_ansi_color(THEME_REC *theme, const char *str,
 			/* ANSI indexed color or RGB color */
 			if (*str != ';') break;
 			str++;
-			for (num2 = 0; i_isdigit(*str); str++)
-				num2 = num2*10 + (*str-'0');
+
+			if (!parse_uint(str, &endptr, 10, &num2)) {
+				return start;
+			}
+			str = endptr;
+
 			if (*str == '\0') return start;
 
 			switch (num2) {
@@ -1006,8 +1014,12 @@ static const char *get_ansi_color(THEME_REC *theme, const char *str,
 				/* indexed */
 				if (*str != ';') break;
 				str++;
-				for (num2 = 0; i_isdigit(*str); str++)
-					num2 = num2*10 + (*str-'0');
+
+				if (!parse_uint(str, &endptr, 10, &num2)) {
+					return start;
+				}
+				str = endptr;
+
 				if (*str == '\0') return start;
 
 				if (num == 38) {