From 2d90add5ad216807ec1433e5367fae730e74a4cb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 28 Jul 2017 15:23:32 -0400 Subject: t5813: add test for hostname starting with dash Per the explanation in the previous patch, this should be (and is) rejected. Signed-off-by: Jeff King Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t5813-proto-disable-ssh.sh | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 't') diff --git a/t/t5813-proto-disable-ssh.sh b/t/t5813-proto-disable-ssh.sh index a954ead8af..0ecdceecd6 100755 --- a/t/t5813-proto-disable-ssh.sh +++ b/t/t5813-proto-disable-ssh.sh @@ -17,4 +17,13 @@ test_proto "host:path" ssh "remote:repo.git" test_proto "ssh://" ssh "ssh://remote$PWD/remote/repo.git" test_proto "git+ssh://" ssh "git+ssh://remote$PWD/remote/repo.git" +# Don't even bother setting up a "-remote" directory, as ssh would generally +# complain about the bogus option rather than completing our request. Our +# fake wrapper actually _can_ handle this case, but it's more robust to +# simply confirm from its output that it did not run at all. +test_expect_success 'hostnames starting with dash are rejected' ' + test_must_fail git clone ssh://-remote/repo.git dash-host 2>stderr && + ! grep ^ssh: stderr +' + test_done -- cgit v1.2.1 From 3be4cf09cd3d0747af3ecdb8dc3962a0969b731e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 28 Jul 2017 15:26:50 -0400 Subject: connect: reject dashed arguments for proxy commands If you have a GIT_PROXY_COMMAND configured, we will run it with the host/port on the command-line. If a URL contains a mischievous host like "--foo", we don't know how the proxy command may handle it. It's likely to break, but it may also do something dangerous and unwanted (technically it could even do something useful, but that seems unlikely). We should err on the side of caution and reject this before we even run the command. The hostname check matches the one we do in a similar circumstance for ssh. The port check is not present for ssh, but there it's not necessary because the syntax is "-p ", and there's no ambiguity on the parsing side. It's not clear whether you can actually get a negative port to the proxy here or not. Doing: git fetch git://remote:-1234/repo.git keeps the "-1234" as part of the hostname, with the default port of 9418. But it's a good idea to keep this check close to the point of running the command to make it clear that there's no way to circumvent it (and at worst it serves as a belt-and-suspenders check). Signed-off-by: Jeff King Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t5532-fetch-proxy.sh | 5 +++++ 1 file changed, 5 insertions(+) (limited to 't') diff --git a/t/t5532-fetch-proxy.sh b/t/t5532-fetch-proxy.sh index 5531bd1af4..d3b2651b61 100755 --- a/t/t5532-fetch-proxy.sh +++ b/t/t5532-fetch-proxy.sh @@ -40,4 +40,9 @@ test_expect_success 'fetch through proxy works' ' test_cmp expect actual ' +test_expect_success 'funny hostnames are rejected before running proxy' ' + test_must_fail git fetch git://-remote/repo.git 2>stderr && + ! grep "proxying for" stderr +' + test_done -- cgit v1.2.1 From aeeb2d496859419ac1ba1da1162d6f3610f7f1f3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 28 Jul 2017 15:28:55 -0400 Subject: connect: reject paths that look like command line options If we get a repo path like "-repo.git", we may try to invoke "git-upload-pack -repo.git". This is going to fail, since upload-pack will interpret it as a set of bogus options. But let's reject this before we even run the sub-program, since we would not want to allow any mischief with repo names that actually are real command-line options. You can still ask for such a path via git-daemon, but there's no security problem there, because git-daemon enters the repo itself and then passes "." on the command line. Signed-off-by: Jeff King Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t5810-proto-disable-local.sh | 23 +++++++++++++++++++++++ t/t5813-proto-disable-ssh.sh | 14 ++++++++++++++ 2 files changed, 37 insertions(+) (limited to 't') diff --git a/t/t5810-proto-disable-local.sh b/t/t5810-proto-disable-local.sh index 563592d8a8..c1ef99b85c 100755 --- a/t/t5810-proto-disable-local.sh +++ b/t/t5810-proto-disable-local.sh @@ -11,4 +11,27 @@ test_expect_success 'setup repository to clone' ' test_proto "file://" file "file://$PWD" test_proto "path" file . +test_expect_success 'setup repo with dash' ' + git init --bare repo.git && + git push repo.git HEAD && + mv repo.git "$PWD/-repo.git" +' + +# This will fail even without our rejection because upload-pack will +# complain about the bogus option. So let's make sure that GIT_TRACE +# doesn't show us even running upload-pack. +# +# We must also be sure to use "fetch" and not "clone" here, as the latter +# actually canonicalizes our input into an absolute path (which is fine +# to allow). +test_expect_success 'repo names starting with dash are rejected' ' + rm -f trace.out && + test_must_fail env GIT_TRACE="$PWD/trace.out" git fetch -- -repo.git && + ! grep upload-pack trace.out +' + +test_expect_success 'full paths still work' ' + git fetch "$PWD/-repo.git" +' + test_done diff --git a/t/t5813-proto-disable-ssh.sh b/t/t5813-proto-disable-ssh.sh index 0ecdceecd6..3f084ee306 100755 --- a/t/t5813-proto-disable-ssh.sh +++ b/t/t5813-proto-disable-ssh.sh @@ -26,4 +26,18 @@ test_expect_success 'hostnames starting with dash are rejected' ' ! grep ^ssh: stderr ' +test_expect_success 'setup repo with dash' ' + git init --bare remote/-repo.git && + git push remote/-repo.git HEAD +' + +test_expect_success 'repo names starting with dash are rejected' ' + test_must_fail git clone remote:-repo.git dash-path 2>stderr && + ! grep ^ssh: stderr +' + +test_expect_success 'full paths still work' ' + git clone "remote:$PWD/remote/-repo.git" dash-path +' + test_done -- cgit v1.2.1