Skip to content

Commit 69f4c95

Browse files
pks-tgitster
authored andcommitted
parse-options: check for overflow when parsing integers
We use `strtol()` to parse the argument of `OPTION_INTEGER` options. And while we do check that the argument was fully parsed, we don't check `errno` at all and thus may not notice cases where `strtol()` fails. Most importantly, this includes the case where the parsed integer does not fit into a `long` at all. The consequence is that we'll happily continue with an invalid value. Fix the bug by checking `errno`. Note that this change alone is not sufficient to detect all possible overflows: `strtol()` returns a `long`, but we end up assigning the value to an `int` and will thus truncate the value. This will be fixed in subsequent patches. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 64c7f6f commit 69f4c95

File tree

2 files changed

+15
-1
lines changed

2 files changed

+15
-1
lines changed

parse-options.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,20 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
185185
if (!*arg)
186186
return error(_("%s expects a numerical value"),
187187
optname(opt, flags));
188+
189+
errno = 0;
188190
*(int *)opt->value = strtol(arg, (char **)&s, 10);
189191
if (*s)
190192
return error(_("%s expects a numerical value"),
191193
optname(opt, flags));
192-
return 0;
194+
if (errno == ERANGE)
195+
return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"),
196+
arg, optname(opt, flags), (intmax_t)LONG_MIN, (intmax_t)LONG_MAX);
197+
if (errno)
198+
return error_errno(_("value %s for %s cannot be parsed"),
199+
arg, optname(opt, flags));
193200

201+
return 0;
194202
case OPTION_MAGNITUDE:
195203
if (unset) {
196204
*(unsigned long *)opt->value = 0;

t/t0040-parse-options.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,4 +783,10 @@ test_expect_success 'magnitude with units but no numbers' '
783783
test_must_be_empty out
784784
'
785785

786+
test_expect_success 'overflowing integer' '
787+
test_must_fail test-tool parse-options --integer 9223372036854775808 >out 2>err &&
788+
test_grep "value .* for option .* not in range" err &&
789+
test_must_be_empty out
790+
'
791+
786792
test_done

0 commit comments

Comments
 (0)