Re: [PATCH] maint: check return of split_cmdline to avoid bad config strings
From: Miklos Vajna <hidden>
Date: 2016-06-15 22:45:24
On Wed, Sep 24, 2008 at 02:10:28AM -0400, Deskin Miller [off-list ref] wrote:
As the testcase demonstrates, it's possible to have split_cmdline return -1 and deallocate any memory it's allocated, if the config string is missing an end quote. In both the cases below, the return isn't checked, causing a pretty immediate segfault.
I think this could go to the commit message.
quoted hunk ↗ jump to hunk
diff --git a/builtin-merge.c b/builtin-merge.c index b280444..dcaf368 100644 --- a/builtin-merge.c +++ b/builtin-merge.c@@ -442,6 +442,8 @@ static int git_merge_config(const char *k, const char *v, void *cb) buf = xstrdup(v); argc = split_cmdline(buf, &argv); + if (argc < 0) + die("Bad branch.%s.mergeoptions string", branch); argv = xrealloc(argv, sizeof(*argv) * (argc + 2)); memmove(argv + 1, argv, sizeof(*argv) * (argc + 1)); argc++;
ACK.
quoted hunk ↗ jump to hunk
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 64567fb..3794d23 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh@@ -741,4 +741,14 @@ test_expect_success 'symlinked configuration' ' ' +test_expect_success 'check split_cmdline return' " + git config alias.split-cmdline-fix 'echo \"' && + git split-cmdline-fix || test \$? = 128 && + echo foo > foo && + git add foo && + git commit -m 'initial commit' && + git config branch.master.mergeoptions 'echo \"' && + git merge master || test \$? = 128 + "
Why don't you use test_must_fail here?
Attachments
- (unnamed) [application/pgp-signature] 197 bytes