git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/5] config: properly range-check integer values

From
Jeff King <peff@peff.net>
Date
Sep 8, 2013, 08:33 UTC
Message-ID
<20130908083308.GB29554@sigill.intra.peff.net>
In-Reply-To
<20130908082724.GA27184@sigill.intra.peff.net>

When we look at a config value as an integer using the git_config_int function, we carefully range-check the value we get and complain if it is out of our range. But the range we compare to is that of a "long", which we then cast to an "int" in the function's return value. This means that on systems where "int" and "long" have different sizes (e.g., LP64 systems), we may pass the range check, but then return nonsense by truncating the value as we cast it to an int.

We can solve this by converting git_parse_long into git_parse_int, and range-checking the "int" range. Nobody actually cared that we used a "long" internally, since the result was truncated anyway. And the only other caller of git_parse_long is git_config_maybe_bool, which should be fine to just use int (though we will now forbid out-of-range nonsense like setting "merge.ff" to "10g" to mean "true", which is probably a good thing).

Signed-off-by: Jeff King <peff@peff.net>
---
I tested this with:
	diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
	index b1a6365..5444952 100755
	--- a/t/t6500-gc.sh
	+++ b/t/t6500-gc.sh
	@@ -25,4 +25,8 @@ test_expect_success 'gc -h with invalid configuration' '
	 	test_i18ngrep "[Uu]sage" broken/usage
	 '
	 
	+test_expect_success 'gc complains about out-of-range integers' '
	+	test_must_fail git -c gc.auto=3g gc --auto
	+'
	+
	 test_done
but that test is horrible for a few reasons:
  1. It depends on knowing that gc.auto uses git_config_int internally.
  2. It passes without the patch on 32-bit platforms.
  3. It won't pass with the patch on ILP64 systems.

We can't use "git config --int" to test it, because it's going to stop using git_config_int later in the series. So I think we should just go without a test.

 config.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/config.c b/config.c
index ee7c1f8..0a65ac2 100644
--- a/config.c
+++ b/config.c
@@ -515,10 +515,10 @@ static int git_parse_long(const char *value, long *ret)
 	return 0;
 }
 
-static int git_parse_long(const char *value, long *ret)
+static int git_parse_int(const char *value, int *ret)
 {
 	intmax_t tmp;
-	if (!git_parse_signed(value, &tmp, maximum_signed_value_of_type(long)))
+	if (!git_parse_signed(value, &tmp, maximum_signed_value_of_type(int)))
 		return 0;
 	*ret = tmp;
 	return 1;
@@ -542,8 +542,8 @@ int git_config_int(const char *name, const char *value)
 
 int git_config_int(const char *name, const char *value)
 {
-	long ret = 0;
-	if (!git_parse_long(value, &ret))
+	int ret;
+	if (!git_parse_int(value, &ret))
 		die_bad_config(name);
 	return ret;
 }
@@ -575,10 +575,10 @@ int git_config_maybe_bool(const char *name, const char *value)
 
 int git_config_maybe_bool(const char *name, const char *value)
 {
-	long v = git_config_maybe_bool_text(name, value);
+	int v = git_config_maybe_bool_text(name, value);
 	if (0 <= v)
 		return v;
-	if (git_parse_long(value, &v))
+	if (git_parse_int(value, &v))
 		return !!v;
 	return -1;
 }
-- 
1.8.4.2.g87d4a77
Previous: Jeff KingNext: Jeff King
Message 19 of 25 in “git-config and large integers”
  1. 0/2 git-config and large integersJeff King, Aug 20, 2013
  2. 1/2 config: properly range-check integer valuesJeff King, Aug 20, 2013
  3. Jonathan NiederAug 20, 2013
  4. Jeff KingAug 21, 2013
  5. Stefan BellerAug 20, 2013
  6. Jeff KingAug 20, 2013
  7. 2/2 teach git-config to output large integersJeff King, Aug 20, 2013
  8. Jonathan NiederAug 20, 2013
  9. Jeff KingAug 21, 2013
  10. Jonathan NiederAug 21, 2013
  11. Jeff KingAug 21, 2013
  12. Jonathan NiederAug 21, 2013
  13. Junio C HamanoAug 20, 2013
  14. Junio C HamanoAug 20, 2013
  15. Jeff KingAug 21, 2013
  16. Jeff KingAug 21, 2013
  17. 0/5 git-config and large integersJeff King, Sep 8, 2013
  18. 1/5 config: factor out integer parsing from range checksJeff King, Sep 8, 2013
  19. 2/5 config: properly range-check integer valuesJeff King, Sep 8, 2013
  20. 3/5 config: set errno in numeric git_parse_* functionsJeff King, Sep 8, 2013
  21. Eric SunshineSep 9, 2013
  22. Jeff KingSep 9, 2013
  23. 4/5 config: make numeric parsing errors more clearJeff King, Sep 8, 2013
  24. 5/5 git-config: always treat --int as 64-bit internallyJeff King, Sep 8, 2013
  25. Junio C HamanoSep 9, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.