diff options
| -rw-r--r-- | credential.c | 7 | ||||
| -rw-r--r-- | fsck.c | 47 | ||||
| -rwxr-xr-x | t/t5550-http-fetch-dumb.sh | 7 | ||||
| -rwxr-xr-x | t/t7416-submodule-dash-url.sh | 32 |
4 files changed, 84 insertions, 9 deletions
diff --git a/credential.c b/credential.c index 7d43359591..aedb64574d 100644 --- a/credential.c +++ b/credential.c @@ -357,8 +357,11 @@ int credential_from_url_gently(struct credential *c, const char *url, * (3) proto://<user>:<pass>@<host>/... */ proto_end = strstr(url, "://"); - if (!proto_end) - return 0; + if (!proto_end) { + if (!quiet) + warning(_("url has no scheme: %s"), url); + return -1; + } cp = proto_end + 3; at = strchr(cp, '@'); colon = strchr(cp, ':'); @@ -972,6 +972,34 @@ static int submodule_url_is_relative(const char *url) } /* + * Count directory components that a relative submodule URL should chop + * from the remote_url it is to be resolved against. + * + * In other words, this counts "../" components at the start of a + * submodule URL. + * + * Returns the number of directory components to chop and writes a + * pointer to the next character of url after all leading "./" and + * "../" components to out. + */ +static int count_leading_dotdots(const char *url, const char **out) +{ + int result = 0; + while (1) { + if (starts_with_dot_dot_slash(url)) { + result++; + url += strlen("../"); + continue; + } + if (starts_with_dot_slash(url)) { + url += strlen("./"); + continue; + } + *out = url; + return result; + } +} +/* * Check whether a transport is implemented by git-remote-curl. * * If it is, returns 1 and writes the URL that would be passed to @@ -1018,15 +1046,30 @@ static int check_submodule_url(const char *url) return -1; if (submodule_url_is_relative(url)) { + char *decoded; + const char *next; + int has_nl; + /* * This could be appended to an http URL and url-decoded; * check for malicious characters. */ - char *decoded = url_decode(url); - int has_nl = !!strchr(decoded, '\n'); + decoded = url_decode(url); + has_nl = !!strchr(decoded, '\n'); + free(decoded); if (has_nl) return -1; + + /* + * URLs which escape their root via "../" can overwrite + * the host field and previous components, resolving to + * URLs like https::example.com/submodule.git that were + * susceptible to CVE-2020-11008. + */ + if (count_leading_dotdots(url, &next) > 0 && + *next == ':') + return -1; } else if (url_to_curl_url(url, &curl_url)) { 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' ' diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh index afdd2553d9..249dc3d1d4 100755 --- a/t/t7416-submodule-dash-url.sh +++ b/t/t7416-submodule-dash-url.sh @@ -60,6 +60,38 @@ test_expect_success 'trailing backslash is handled correctly' ' test_i18ngrep ! "unknown option" err ' +test_expect_success 'fsck rejects missing URL scheme' ' + git checkout --orphan missing-scheme && + cat >.gitmodules <<-\EOF && + [submodule "foo"] + url = http::one.example.com/foo.git + EOF + git add .gitmodules && + test_tick && + git commit -m "gitmodules with missing 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 missing scheme' ' + git checkout --orphan relative-missing-scheme && + cat >.gitmodules <<-\EOF && + [submodule "foo"] + url = "..\\../.\\../:one.example.com/foo.git" + EOF + git add .gitmodules && + test_tick && + git commit -m "gitmodules with relative URL that strips off 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 && |