maint: check return of split_cmdline to avoid bad config strings

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 <deskinm@umich.edu>
Acked-by: Miklos Vajna <vmiklos@frugalware.org>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/builtin-merge.c b/builtin-merge.c
index b280444e10d67355da6fd0d9e1a2dd2d7a29d440..dcaf3681dc4433a0a258010f2b0a06fc26dc2212 100644 (file)
--- 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);
 
                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++;
                argv = xrealloc(argv, sizeof(*argv) * (argc + 2));
                memmove(argv + 1, argv, sizeof(*argv) * (argc + 1));
                argc++;

diff --git a/git.c b/git.c
index fdb0f71019a02e310e15734193c7556860956115..5582c515ac04609a338de1d2d5e510e7e7c4914d 100644 (file)
--- 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);
                            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"
                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
index 64567fb94d5c3f9587b643333212cdb37a4661ac..11b82f43dd0220c736dd269b2f6531a1381edf3a 100755 (executable)
--- 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
 test_done