CVE-2020-11008: Insufficiently Protected Credentials (CWE-522)
value: Affected versions of Git have a vulnerability whereby Git can be tricked into sending private credentials to a host controlled by an attacker. This bug is similar to CVE-2020-5260(GHSA-qm7j-c969-7j4q). The fix for that bug still left the door open for an exploit where _some_ credential is leaked (but the attacker cannot control which one). Git uses external "credential helper" programs to store and retrieve passwords or other credentials from secure storage provided by the operating system. Specially-crafted URLs that are considered illegal as of the recently published Git versions can cause Git to send a "blank" pattern to helpers, missing hostname and protocol fields. Many helpers will interpret this as matching _any_ URL, and will return some unspecified stored password, leaking the password to an attacker's server. The vulnerability can be triggered by feeding a malicious URL to `git clone`. However, the affected URLs look rather suspicious; the likely vector would be through systems which automatically clone URLs not visible to the user, such as Git submodules, or package systems built around Git. The root of the problem is in Git itself, which should not be feeding blank input to helpers. However, the ability to exploit the vulnerability in practice depends on which helpers are in use. Credential helpers which are known to trigger the vulnerability: - Git's "store" helper - Git's "cache" helper - the "osxkeychain" helper that ships in Git's "contrib" directory Credential helpers which are known to be safe even with vulnerable versions of Git: - Git Credential Manager for Windows Any helper not in this list should be assumed to trigger the vulnerability.
CWE description: The product transmits or stores authentication credentials, but it uses an insecure method that is susceptible to unauthorized interception and/or retrieval.
CVSS v3 base score: 7.5
Published: 2020-04-21T19:15Z
Repository: https://github.com/git/git
Language: C
Files changed: credential.c, fsck.c, t/t5550-http-fetch-dumb.sh, t/t7416-submodule-dash-url.sh
Commit date: 2020-04-19 16:10:58 -0700
Commit message: 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 <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>

Diff: diff --git a/credential.c b/credential.c
index 7d433595913..aedb64574de 100644
--- a/credential.c
+++ b/credential.c
@@ -355,12 +355,15 @@ int credential_from_url_gently(struct credential *c, const char *url,
 	 *   (1) proto://<host>/...
 	 *   (2) proto://<user>@<host>/...
 	 *   (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, ':');
 	slash = strchrnul(cp, '/');
 
diff --git a/fsck.c b/fsck.c
index 4e3bc8622f3..41af5c0d5f5 100644
--- a/fsck.c
+++ b/fsck.c
@@ -969,10 +969,38 @@ static int starts_with_dot_dot_slash(const char *str)
 static int submodule_url_is_relative(const char *url)
 {
 	return starts_with_dot_slash(url) || starts_with_dot_dot_slash(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
  * git-remote-curl to the "out" parameter.
@@ -1016,19 +1044,34 @@ static int check_submodule_url(const char *url)
 
 	if (looks_like_command_line_option(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)) {
 		struct credential c = CREDENTIAL_INIT;
 		int ret = credential_from_url_gently(&c, curl_url, 1);
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 8552184e741..517202e4772 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -308,15 +308,12 @@ test_expect_success 'git client does not send an empty Accept-Language' '
 	GIT_TRACE_CURL=true LANGUAGE= git ls-remote "$HTTPD_URL/dumb/repo.git" 2>stderr &&
 	! grep "^=> Send header: Accept-Language:" stderr
 '
 
 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' '
 	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 afdd2553d91..249dc3d1d46 100755
--- a/t/t7416-submodule-dash-url.sh
+++ b/t/t7416-submodule-dash-url.sh
@@ -58,10 +58,42 @@ test_expect_success 'trailing backslash is handled correctly' '
 	git commit -am "Add testmodule" &&
 	test_must_fail git clone --verbose --recurse-submodules . dolly 2>err &&
 	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 &&
 	[submodule "foo"]
 		url = "data://acjbkd%0akajfdickajkd"

Vulnerable code: 	if (!proto_end)
		return 0;
		char *decoded = url_decode(url);
		int has_nl = !!strchr(decoded, '\n');
	# 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
Fixed code: 	if (!proto_end) {
		if (!quiet)
			warning(_("url has no scheme: %s"), url);
		return -1;
	}
/*
 * 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;
	}
}
		char *decoded;
		const char *next;
		int has_nl;

		decoded = url_decode(url);
		has_nl = !!strchr(decoded, '\n');


		/*
		 * 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;
	test_must_fail git remote-http http::/example.com/repo.git 2>stderr &&
	test_i18ngrep "url has no scheme" stderr
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
'