maint: check return of split_cmdline to avoid bad config strings

deskin · spearce · commit dc4179f9a764 · 2008-09-24T08:58:14.000-07:00
As the testcase demonstrates, it's possible for split_cmdline to return -1 and
deallocate any memory it's allocated, if the config string is missing an end
quote.  In both the cases below, which are the only calling sites, the return
isn't checked, and using the pointer causes a pretty immediate segfault.

Signed-off-by: Deskin Miller &lt;deskinm@umich.edu&gt;
Acked-by: Miklos Vajna &lt;vmiklos@frugalware.org&gt;
Signed-off-by: Shawn O. Pearce &lt;spearce@spearce.org&gt;
diff --git 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++;
diff --git a/git.c b/git.c
@@ -162,6 +162,8 @@ static int handle_alias(int *argcp, const char ***argv)
 			    alias_string + 1, alias_command);
 		}
 		count = split_cmdline(alias_string, &new_argv);
+		if (count < 0)
+			die("Bad alias.%s string", alias_command);
 		option_count = handle_options(&new_argv, &count, &envchanged);
 		if (envchanged)
 			die("alias '%s' changes environment variables\n"
diff --git 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 \"' &&
+	test_must_fail git split-cmdline-fix &&
+	echo foo > foo &&
+	git add foo &&
+	git commit -m 'initial commit' &&
+	git config branch.master.mergeoptions 'echo \"' &&
+	test_must_fail git merge master
+	"
+
 test_done

Original file line number	Diff line number	Diff line change
`@@ -162,6 +162,8 @@ static int handle_alias(int argcp, const char **argv)`
`162`	`162`	`alias_string + 1, alias_command);`
`163`	`163`	`}`
`164`	`164`	`count = split_cmdline(alias_string, &new_argv);`
	`165`	`+ if (count < 0)`
	`166`	`+ die("Bad alias.%s string", alias_command);`
`165`	`167`	`option_count = handle_options(&new_argv, &count, &envchanged);`
`166`	`168`	`if (envchanged)`
`167`	`169`	`die("alias '%s' changes environment variables\n"`