git_parse_signed(): avoid integer overflow

git_parse_signed() checks that the absolute value of the parsed string
is less than or equal to a caller supplied maximum value. When
calculating the absolute value there is a integer overflow if `val ==
INTMAX_MIN`. To fix this avoid negating `val` when it is negative by
having separate overflow checks for positive and negative values.

An alternative would be to special case INTMAX_MIN before negating `val`
as it is always out of range. That would enable us to keep the existing
code but I'm not sure that the current two-stage check is any clearer
than the new version.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Taylor Blau <me@ttaylorr.com>

diff --git a/config.c b/config.c
index b7fb68026d84088da49cb42797cece8d23f4b535..aad3e00341d7401af173f3407de6025f7d2eb850 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1160,8 +1160,10 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
        if (value && *value) {
                char *end;
                intmax_t val;
-               uintmax_t uval;
-               uintmax_t factor;
+               intmax_t factor;
+
+               if (max < 0)
+                       BUG("max must be a positive integer");
 
                errno = 0;
                val = strtoimax(value, &end, 0);
@@ -1176,9 +1178,8 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
                        errno = EINVAL;
                        return 0;
                }
-               uval = val < 0 ? -val : val;
-               if (unsigned_mult_overflows(factor, uval) ||
-                   factor * uval > max) {
+               if ((val < 0 && -max / factor > val) ||
+                   (val > 0 && max / factor < val)) {
                        errno = ERANGE;
                        return 0;
                }