git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Ben Walton <bwalton@artsci.utoronto.ca>
To: gitster@pobox.com, git@vger.kernel.org
Cc: Ben Walton <bwalton@artsci.utoronto.ca>
Subject: [PATCH] Prevent git-config from storing section keys that are too long
Date: Thu,  6 Sep 2012 20:47:09 -0400	[thread overview]
Message-ID: <1346978829-4486-1-git-send-email-bwalton@artsci.utoronto.ca> (raw)

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@artsci.utoronto.ca>
---

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

             reply	other threads:[~2012-09-07  0:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-07  0:47 Ben Walton [this message]
2012-09-07  1:33 ` [PATCH] Prevent git-config from storing section keys that are too long Junio C Hamano
2012-09-07  2:34   ` Ben Walton
2012-09-29 10:19   ` [PATCH] Remove the hard coded length limit on variable names in config files Ben Walton
2012-09-30  4:05     ` Michael Haggerty
2012-09-30 18:20       ` Ben Walton
2012-09-30 19:44         ` Ben Walton
2012-10-01 19:33           ` Junio C Hamano
2012-10-01  3:16         ` Michael Haggerty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1346978829-4486-1-git-send-email-bwalton@artsci.utoronto.ca \
    --to=bwalton@artsci.utoronto.ca \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).