Skip to content

Commit 035b737

Browse files
committed
Merge branch 'jk/ssh-funny-url'
This topic branch brings a critical bug fix to the Git for Windows v2.13.1 "maintenance track". Signed-off-by: Johannes Schindelin <[email protected]>
2 parents a36e14b + aeeb2d4 commit 035b737

File tree

7 files changed

+79
-0
lines changed

7 files changed

+79
-0
lines changed

cache.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,14 @@ char *strip_path_suffix(const char *path, const char *suffix);
11831183
int daemon_avoid_alias(const char *path);
11841184
extern int is_ntfs_dotgit(const char *name);
11851185

1186+
/*
1187+
* Returns true iff "str" could be confused as a command-line option when
1188+
* passed to a sub-program like "ssh". Note that this has nothing to do with
1189+
* shell-quoting, which should be handled separately; we're assuming here that
1190+
* the string makes it verbatim to the sub-program.
1191+
*/
1192+
int looks_like_command_line_option(const char *str);
1193+
11861194
/**
11871195
* Return a newly allocated string with the evaluation of
11881196
* "$XDG_CONFIG_HOME/git/$filename" if $XDG_CONFIG_HOME is non-empty, otherwise

connect.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,11 @@ static struct child_process *git_proxy_connect(int fd[2], char *host)
577577

578578
get_host_and_port(&host, &port);
579579

580+
if (looks_like_command_line_option(host))
581+
die("strange hostname '%s' blocked", host);
582+
if (looks_like_command_line_option(port))
583+
die("strange port '%s' blocked", port);
584+
580585
proxy = xmalloc(sizeof(*proxy));
581586
child_process_init(proxy);
582587
argv_array_push(&proxy->args, git_proxy_command);
@@ -823,6 +828,9 @@ struct child_process *git_connect(int fd[2], const char *url,
823828
conn = xmalloc(sizeof(*conn));
824829
child_process_init(conn);
825830

831+
if (looks_like_command_line_option(path))
832+
die("strange pathname '%s' blocked", path);
833+
826834
strbuf_addstr(&cmd, prog);
827835
strbuf_addch(&cmd, ' ');
828836
sq_quote_buf(&cmd, path);
@@ -856,6 +864,9 @@ struct child_process *git_connect(int fd[2], const char *url,
856864
return NULL;
857865
}
858866

867+
if (looks_like_command_line_option(ssh_host))
868+
die("strange hostname '%s' blocked", ssh_host);
869+
859870
ssh = get_ssh_command();
860871
if (ssh)
861872
handle_ssh_variant(ssh, 1, &port_option,

path.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,6 +1246,11 @@ int is_ntfs_dotgit(const char *name)
12461246
}
12471247
}
12481248

1249+
int looks_like_command_line_option(const char *str)
1250+
{
1251+
return str && str[0] == '-';
1252+
}
1253+
12491254
char *xdg_config_home(const char *filename)
12501255
{
12511256
const char *home, *config_home;

t/lib-proto-disable.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ test_proto () {
182182
test_whitelist "$@"
183183

184184
test_config "$@"
185+
186+
test_expect_success 'unset protocol.allow' '
187+
git config --global --unset protocol.allow
188+
'
185189
}
186190

187191
# set up an ssh wrapper that will access $host/$repo in the

t/t5532-fetch-proxy.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,9 @@ test_expect_success 'fetch through proxy works' '
4343
test_cmp expect actual
4444
'
4545

46+
test_expect_success 'funny hostnames are rejected before running proxy' '
47+
test_must_fail git fetch git://-remote/repo.git 2>stderr &&
48+
! grep "proxying for" stderr
49+
'
50+
4651
test_done

t/t5810-proto-disable-local.sh

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,27 @@ test_expect_success 'setup repository to clone' '
1111
test_proto "file://" file "file://$PWD"
1212
test_proto "path" file .
1313

14+
test_expect_success 'setup repo with dash' '
15+
git init --bare repo.git &&
16+
git push repo.git HEAD &&
17+
mv repo.git "$PWD/-repo.git"
18+
'
19+
20+
# This will fail even without our rejection because upload-pack will
21+
# complain about the bogus option. So let's make sure that GIT_TRACE
22+
# doesn't show us even running upload-pack.
23+
#
24+
# We must also be sure to use "fetch" and not "clone" here, as the latter
25+
# actually canonicalizes our input into an absolute path (which is fine
26+
# to allow).
27+
test_expect_success 'repo names starting with dash are rejected' '
28+
rm -f trace.out &&
29+
test_must_fail env GIT_TRACE="$PWD/trace.out" git fetch -- -repo.git &&
30+
! grep upload-pack trace.out
31+
'
32+
33+
test_expect_success 'full paths still work' '
34+
git fetch "$PWD/-repo.git"
35+
'
36+
1437
test_done

t/t5813-proto-disable-ssh.sh

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,27 @@ test_proto "host:path" ssh "remote:repo.git"
1717
test_proto "ssh://" ssh "ssh://remote$PWD/remote/repo.git"
1818
test_proto "git+ssh://" ssh "git+ssh://remote$PWD/remote/repo.git"
1919

20+
# Don't even bother setting up a "-remote" directory, as ssh would generally
21+
# complain about the bogus option rather than completing our request. Our
22+
# fake wrapper actually _can_ handle this case, but it's more robust to
23+
# simply confirm from its output that it did not run at all.
24+
test_expect_success 'hostnames starting with dash are rejected' '
25+
test_must_fail git clone ssh://-remote/repo.git dash-host 2>stderr &&
26+
! grep ^ssh: stderr
27+
'
28+
29+
test_expect_success 'setup repo with dash' '
30+
git init --bare remote/-repo.git &&
31+
git push remote/-repo.git HEAD
32+
'
33+
34+
test_expect_success 'repo names starting with dash are rejected' '
35+
test_must_fail git clone remote:-repo.git dash-path 2>stderr &&
36+
! grep ^ssh: stderr
37+
'
38+
39+
test_expect_success 'full paths still work' '
40+
git clone "remote:$PWD/remote/-repo.git" dash-path
41+
'
42+
2043
test_done

0 commit comments

Comments
 (0)