[PATCH v3 2/7] parse-options: check for overflow when parsing integers
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Apr 16, 2025, 10:02 UTC
- Message-ID
- <20250416-b4-pks-parse-options-integers-v3-2-d390746bea79@pks.im>
- In-Reply-To
- <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im>
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 <ps@pks.im> --- parse-options.c | 10 +++++++++- t/t0040-parse-options.sh | 6 ++++++ 2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/parse-options.c b/parse-options.c index 35fbb3b0d63..e8c08e55e02 100644 --- a/parse-options.c +++ b/parse-options.c @@ -185,12 +185,20 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, if (!*arg) return error(_("%s expects a numerical value"), optname(opt, flags)); + + errno = 0; *(int *)opt->value = strtol(arg, (char **)&s, 10); if (*s) return error(_("%s expects a numerical value"), optname(opt, flags)); - return 0; + if (errno == ERANGE) + return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"), + arg, optname(opt, flags), (intmax_t)LONG_MIN, (intmax_t)LONG_MAX); + if (errno) + return error_errno(_("value %s for %s cannot be parsed"), + arg, optname(opt, flags)); + return 0; case OPTION_MAGNITUDE: if (unset) { *(unsigned long *)opt->value = 0; diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 2fe3522305f..5eb1feb61b4 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -783,4 +783,10 @@ test_expect_success 'magnitude with units but no numbers' ' test_must_be_empty out ' +test_expect_success 'overflowing integer' ' + test_must_fail test-tool parse-options --integer 9223372036854775808 >out 2>err && + test_grep "value .* for option .* not in range" err && + test_must_be_empty out +' + test_done
-- 2.49.0.805.g082f7c87e0.dirty