[v3,2/7] parse-options: check for overflow when parsing integers

Commit Message

Patrick Steinhardt April 16, 2025, 10:02 a.m. UTC

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 <[e-mail]>
---
 parse-options.c          | 10 +++++++++-
 t/t0040-parse-options.sh |  6 ++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

Patch

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