connect.c: handle errors from split_cmdline

Commit e9d9a8a4d (connect: handle putty/plink also in
GIT_SSH_COMMAND, 2017-01-02) added a call to
split_cmdline(), but checks only for a non-zero return to
see if we got any output. Since the function returns
negative values (and a NULL argv) on error, we end up
dereferencing NULL and segfaulting.

Arguably we could report on the parsing error here, but it's
probably not worth it. This is a best-effort attempt to see
if we are using plink. So we can simply return here with
"no, it wasn't plink" and let the shell actually complain
about the bogus quoting.

Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/connect.c b/connect.c
index 7d65c1c73634363e85a652619cc8705223bc70cb..6f2281ab0f988ffba9d5fa00809d27287bcb3810 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -730,7 +730,7 @@ static void handle_ssh_variant(const char *ssh_command, int is_cmdline,
                const char **ssh_argv;
 
                p = xstrdup(ssh_command);
-               if (split_cmdline(p, &ssh_argv)) {
+               if (split_cmdline(p, &ssh_argv) > 0) {
                        variant = basename((char *)ssh_argv[0]);
                        /*
                         * At this point, variant points into the buffer

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index b52b8acf9859b0084ea10ebf184ed95cafb0b2d8..9c56f771b619e4bb931ce3d34a5f9325b8f61e90 100755 (executable)
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -427,6 +427,12 @@ test_expect_success 'GIT_SSH_VARIANT overrides plink to tortoiseplink' '
        expect_ssh "-batch -P 123" myhost src
 '
 
+test_expect_success 'clean failure on broken quoting' '
+       test_must_fail \
+               env GIT_SSH_COMMAND="${SQ}plink.exe -v" \
+               git clone "[myhost:123]:src" sq-failure
+'
+
 # Reset the GIT_SSH environment variable for clone tests.
 setup_ssh_wrapper