From c44088ecc4b0722636e0a305f9608d3047197282 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 18 Apr 2020 20:54:13 -0700 Subject: credential: treat URL without scheme as invalid libcurl permits making requests without a URL scheme specified. In this case, it guesses the URL from the hostname, so I can run git ls-remote http::ftp.example.com/path/to/repo and it would make an FTP request. Any user intentionally using such a URL is likely to have made a typo. Unfortunately, credential_from_url is not able to determine the host and protocol in order to determine appropriate credentials to send, and until "credential: refuse to operate when missing host or protocol", this resulted in another host's credentials being leaked to the named host. Teach credential_from_url_gently to consider such a URL to be invalid so that fsck can detect and block gitmodules files with such URLs, allowing server operators to avoid serving them to downstream users running older versions of Git. This also means that when such URLs are passed on the command line, Git will print a clearer error so affected users can switch to the simpler URL that explicitly specifies the host and protocol they intend. One subtlety: .gitmodules files can contain relative URLs, representing a URL relative to the URL they were cloned from. The relative URL resolver used for .gitmodules can follow ".." components out of the path part and past the host part of a URL, meaning that such a relative URL can be used to traverse from a https://foo.example.com/innocent superproject to a https::attacker.example.com/exploit submodule. Fortunately a leading ':' in the first path component after a series of leading './' and '../' components is unlikely to show up in other contexts, so we can catch this by detecting that pattern. Reported-by: Jeff King Signed-off-by: Jonathan Nieder Reviewed-by: Jeff King --- t/t5550-http-fetch-dumb.sh | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 't/t5550-http-fetch-dumb.sh') diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 8552184e74..517202e477 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -310,11 +310,8 @@ test_expect_success 'git client does not send an empty Accept-Language' ' ' test_expect_success 'remote-http complains cleanly about malformed urls' ' - # do not actually issue "list" or other commands, as we do not - # want to rely on what curl would actually do with such a broken - # URL. This is just about making sure we do not segfault during - # initialization. - test_must_fail git remote-http http::/example.com/repo.git + test_must_fail git remote-http http::/example.com/repo.git 2>stderr && + test_i18ngrep "url has no scheme" stderr ' test_expect_success 'redirects can be forbidden/allowed' ' -- cgit v1.2.3 From e7fab62b736cca3416660636e46f0be8386a5030 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 18 Apr 2020 20:54:57 -0700 Subject: credential: treat URL with empty scheme as invalid Until "credential: refuse to operate when missing host or protocol", Git's credential handling code interpreted URLs with empty scheme to mean "give me credentials matching this host for any protocol". Luckily libcurl does not recognize such URLs (it tries to look for a protocol named "" and fails). Just in case that changes, let's reject them within Git as well. This way, credential_from_url is guaranteed to always produce a "struct credential" with protocol and host set. Signed-off-by: Jonathan Nieder --- credential.c | 5 ++--- t/t5550-http-fetch-dumb.sh | 9 +++++++++ t/t7416-submodule-dash-url.sh | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) (limited to 't/t5550-http-fetch-dumb.sh') diff --git a/credential.c b/credential.c index aedb64574d..64a841eddc 100644 --- a/credential.c +++ b/credential.c @@ -357,7 +357,7 @@ int credential_from_url_gently(struct credential *c, const char *url, * (3) proto://:@/... */ proto_end = strstr(url, "://"); - if (!proto_end) { + if (!proto_end || proto_end == url) { if (!quiet) warning(_("url has no scheme: %s"), url); return -1; @@ -382,8 +382,7 @@ int credential_from_url_gently(struct credential *c, const char *url, host = at + 1; } - if (proto_end - url > 0) - c->protocol = xmemdupz(url, proto_end - url); + c->protocol = xmemdupz(url, proto_end - url); c->host = url_decode_mem(host, slash - host); /* Trim leading and trailing slashes from path */ while (*slash == '/') diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 517202e477..86f6eeaf71 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -314,6 +314,15 @@ test_expect_success 'remote-http complains cleanly about malformed urls' ' test_i18ngrep "url has no scheme" stderr ' +# NEEDSWORK: Writing commands to git-remote-curl can race against the latter +# erroring out, producing SIGPIPE. Remove "ok=sigpipe" once transport-helper has +# learned to handle early remote helper failures more cleanly. +test_expect_success 'remote-http complains cleanly about empty scheme' ' + test_must_fail ok=sigpipe git ls-remote \ + http::${HTTPD_URL#http}/dumb/repo.git 2>stderr && + test_i18ngrep "url has no scheme" stderr +' + test_expect_success 'redirects can be forbidden/allowed' ' test_must_fail git -c http.followRedirects=false \ clone $HTTPD_URL/dumb-redir/repo.git dumb-redir && diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh index 249dc3d1d4..9309040373 100755 --- a/t/t7416-submodule-dash-url.sh +++ b/t/t7416-submodule-dash-url.sh @@ -92,6 +92,38 @@ test_expect_success 'fsck rejects relative URL resolving to missing scheme' ' grep gitmodulesUrl err ' +test_expect_success 'fsck rejects empty URL scheme' ' + git checkout --orphan empty-scheme && + cat >.gitmodules <<-\EOF && + [submodule "foo"] + url = http::://one.example.com/foo.git + EOF + git add .gitmodules && + test_tick && + git commit -m "gitmodules with empty URL scheme" && + test_when_finished "rm -rf dst" && + git init --bare dst && + git -C dst config transfer.fsckObjects true && + test_must_fail git push dst HEAD 2>err && + grep gitmodulesUrl err +' + +test_expect_success 'fsck rejects relative URL resolving to empty scheme' ' + git checkout --orphan relative-empty-scheme && + cat >.gitmodules <<-\EOF && + [submodule "foo"] + url = ../../../:://one.example.com/foo.git + EOF + git add .gitmodules && + test_tick && + git commit -m "relative gitmodules URL resolving to empty scheme" && + test_when_finished "rm -rf dst" && + git init --bare dst && + git -C dst config transfer.fsckObjects true && + test_must_fail git push dst HEAD 2>err && + grep gitmodulesUrl err +' + test_expect_success 'fsck permits embedded newline with unrecognized scheme' ' git checkout --orphan newscheme && cat >.gitmodules <<-\EOF && -- cgit v1.2.3