Skip to content

Commit 14770cf

Browse files
phillipwoodttaylorr
authored andcommitted
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 <[email protected]> Signed-off-by: Taylor Blau <[email protected]>
1 parent 7595c0e commit 14770cf

File tree

1 file changed

+6
-5
lines changed

1 file changed

+6
-5
lines changed

config.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,8 +1160,10 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
11601160
if (value && *value) {
11611161
char *end;
11621162
intmax_t val;
1163-
uintmax_t uval;
1164-
uintmax_t factor;
1163+
intmax_t factor;
1164+
1165+
if (max < 0)
1166+
BUG("max must be a positive integer");
11651167

11661168
errno = 0;
11671169
val = strtoimax(value, &end, 0);
@@ -1176,9 +1178,8 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
11761178
errno = EINVAL;
11771179
return 0;
11781180
}
1179-
uval = val < 0 ? -val : val;
1180-
if (unsigned_mult_overflows(factor, uval) ||
1181-
factor * uval > max) {
1181+
if ((val < 0 && -max / factor > val) ||
1182+
(val > 0 && max / factor < val)) {
11821183
errno = ERANGE;
11831184
return 0;
11841185
}

0 commit comments

Comments
 (0)