[PATCH] Prevent git-config from storing section keys that are too long

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Key names have a length limit defined by MAXNAME in config.c.  When
reading the config file, we reserve half of this limit for the section
identifier and the other half for the key name within that section.

For example, if setting a key named url.foo.insteadOf, url.foo may use
at most half of MANXNAME.

The parser will throw an error if this condition is violated.

This patch ensures that git-config enforces the same restriction
during the creation of a section identifier so that it doesn't allow
the generate a configuration file that cannot be re-read later.

This patch also adds a test to t1303-wacky-config to catch any future
issues with this check.

Signed-off-by: Ben Walton <bwalton@xxxxxxxxxxxxxxxxxx>
---

Hi All,

I happened to notice this while running the test suite in a deeply
nested directory...

The check for baselen exceeding half of MAXNAME could be done earlier
in the function but doing it late allowed the error message to be
clearer without extra hassle.

I also wonder if MAXNAME should be increased somewhat.  Section
identifiers generated from keys like:

url./some/really/long/path.insteadOf

could overrun the current limit.  It's not a common case, of course,
or this issue would have been found sooner.  Would doubling the
current limit be out of the question?

Thanks
-Ben



 config.c                |    8 ++++++++
 t/t1303-wacky-config.sh |    4 ++++
 2 files changed, 12 insertions(+)

diff --git a/config.c b/config.c
index 2b706ea..d3f4854 100644
--- a/config.c
+++ b/config.c
@@ -1276,6 +1276,14 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_)
 	}
 	(*store_key)[i] = 0;
 
+	if (baselen > MAXNAME / 2) {
+		/* ok to destroy this value now since it will be freed */
+		(*store_key)[baselen] = '\0';
+		error("section identifier for key is too long (> %d): %s",
+		      MAXNAME / 2, *store_key);
+		goto out_free_ret_1;
+	}
+
 	return 0;
 
 out_free_ret_1:
diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
index 46103a1..12f0850 100755
--- a/t/t1303-wacky-config.sh
+++ b/t/t1303-wacky-config.sh
@@ -47,4 +47,8 @@ test_expect_success 'do not crash on special long config line' '
 	check section.key "$LONG_VALUE"
 '
 
+test_expect_success 'do not accept long section identifiers for key names' '
+	test_must_fail git config some.REALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlong.key value
+'
+
 test_done
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]