Merge branch 'pw/config-int-parse-fixes' into jch

Assorted fixes of parsing end-user input as integers.

* pw/config-int-parse-fixes:
  git_parse_signed(): avoid integer overflow
  config: require at least one digit when parsing numbers
  git_parse_unsigned: reject negative values

4 files changed, 43 insertions, 5 deletions

diff --git a/config.c b/config.c
index c058b2c70c..27f38283ad 100644
--- a/config.c
+++ b/config.c
@@ -1160,21 +1160,26 @@ 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);
 		if (errno == ERANGE)
 			return 0;
+		if (end == value) {
+			errno = EINVAL;
+			return 0;
+		}
 		factor = get_unit_factor(end);
 		if (!factor) {
 			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;
 		}
@@ -1193,10 +1198,19 @@ static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max)
 		uintmax_t val;
 		uintmax_t factor;
 
+		/* negative values would be accepted by strtoumax */
+		if (strchr(value, '-')) {
+			errno = EINVAL;
+			return 0;
+		}
 		errno = 0;
 		val = strtoumax(value, &end, 0);
 		if (errno == ERANGE)
 			return 0;
+		if (end == value) {
+			errno = EINVAL;
+			return 0;
+		}
 		factor = get_unit_factor(end);
 		if (!factor) {
 			errno = EINVAL;
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 5cc62306e3..7d7ecfd571 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -709,4 +709,16 @@ test_expect_success 'subcommands are incompatible with KEEP_DASHDASH unless in c
 	grep ^BUG err
 '
 
+test_expect_success 'negative magnitude' '
+	test_must_fail test-tool parse-options --magnitude -1 >out 2>err &&
+	grep "non-negative integer" err &&
+	test_must_be_empty out
+'
+
+test_expect_success 'magnitude with units but no numbers' '
+	test_must_fail test-tool parse-options --magnitude m >out 2>err &&
+	grep "non-negative integer" err &&
+	test_must_be_empty out
+'
+
 test_done
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index 4f3aa17c99..c71932b024 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -5,6 +5,12 @@ test_description='adding and checking out large blobs'
 
 . ./test-lib.sh
 
+test_expect_success 'core.bigFileThreshold must be non-negative' '
+	test_must_fail git -c core.bigFileThreshold=-1 rev-parse >out 2>err &&
+	grep "bad numeric config value" err &&
+	test_must_be_empty out
+'
+
 test_expect_success setup '
 	# clone does not allow us to pass core.bigfilethreshold to
 	# new repos, so set core.bigfilethreshold globally
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index c6661e61af..2575279ab8 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2228,6 +2228,12 @@ test_expect_success '--type rejects unknown specifiers' '
 	test_i18ngrep "unrecognized --type argument" error
 '
 
+test_expect_success '--type=int requires at least one digit' '
+	test_must_fail git config --type int --default m some.key >out 2>error &&
+	grep "bad numeric config value" error &&
+	test_must_be_empty out
+'
+
 test_expect_success '--replace-all does not invent newlines' '
 	q_to_tab >.git/config <<-\EOF &&
 	[abc]key