Version in base suite: 2.20.1-2+deb10u1 Version in overlay suite: 2.20.1-2+deb10u2 Base version: git_2.20.1-2+deb10u2 Target version: git_2.20.1-2+deb10u3 Base file: /srv/ftp-master.debian.org/ftp/pool/main/g/git/git_2.20.1-2+deb10u2.dsc Target file: /srv/ftp-master.debian.org/policy/pool/main/g/git/git_2.20.1-2+deb10u3.dsc changelog | 29 + patches/0048-t0300-make-quit-helper-more-realistic.diff | 67 ++ patches/0049-t0300-use-more-realistic-inputs.diff | 289 ++++++++++ patches/0050-credential-parse-URL-without-host-as-empty-host-not-u.diff | 94 +++ patches/0051-credential-refuse-to-operate-when-missing-host-or-pro.diff | 170 +++++ patches/0052-fsck-convert-gitmodules-url-to-URL-passed-to-curl.diff | 209 +++++++ patches/0053-credential-die-when-parsing-invalid-urls.diff | 82 ++ patches/0054-credential-treat-URL-without-scheme-as-invalid.diff | 204 +++++++ patches/0055-credential-treat-URL-with-empty-scheme-as-invalid.diff | 112 +++ patches/0056-fsck-reject-URL-with-empty-host-in-.gitmodules.diff | 112 +++ patches/0057-Git-2.17.5.diff | 45 + patches/0058-Git-2.18.4.diff | 29 + patches/0059-Git-2.19.5.diff | 29 + patches/0060-Git-2.20.4.diff | 43 + patches/series | 13 15 files changed, 1527 insertions(+) diff -Nru git-2.20.1/debian/changelog git-2.20.1/debian/changelog --- git-2.20.1/debian/changelog 2020-04-12 07:24:43.000000000 +0000 +++ git-2.20.1/debian/changelog 2020-04-20 00:19:12.000000000 +0000 @@ -1,3 +1,32 @@ +git (1:2.20.1-2+deb10u3) buster-security; urgency=high + + * new upstream point release (see RelNotes/2.20.4.txt). + * Addresses the security issue CVE-2020-11008. + + With a crafted URL that contains a newline or empty host, or + lacks a scheme, the credential helper machinery can be fooled + into providing credential information that is not appropriate + for the protocol in use and host being contacted. + + Unlike the vulnerability fixed in 1:2.20.1-2+deb10u2, the + credentials are not for a host of the attacker's choosing. + Instead, they are for an unspecified host, based on how the + configured credential helper handles an absent "host" + parameter. + + The attack has been made impossible by refusing to work with + underspecified credential patterns. + + Thanks to Carlo Arenas for reporting that Git was still + vulnerable, Felix Wilhelm for providing the proof of concept + demonstrating this issue, and Jeff King for promptly providing + a corrected fix. + + Tested using the proof of concept at + https://crbug.com/project-zero/2021. + + -- Jonathan Nieder Sun, 19 Apr 2020 17:19:12 -0700 + git (1:2.20.1-2+deb10u2) buster-security; urgency=high [ Salvatore Bonaccorso ] diff -Nru git-2.20.1/debian/patches/0048-t0300-make-quit-helper-more-realistic.diff git-2.20.1/debian/patches/0048-t0300-make-quit-helper-more-realistic.diff --- git-2.20.1/debian/patches/0048-t0300-make-quit-helper-more-realistic.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0048-t0300-make-quit-helper-more-realistic.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,67 @@ +From 4246c942779cc97bb4042afeccd32879c0bacce8 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sun, 19 Apr 2020 01:36:02 -0400 +Subject: t0300: make "quit" helper more realistic + +We test a toy credential helper that writes "quit=1" and confirms that +we stop running other helpers. However, that helper is unrealistic in +that it does not bother to read its stdin at all. + +For now we don't send any input to it, because we feed git-credential a +blank credential. But that will change in the next patch, which will +cause this test to racily fail, as git-credential will get SIGPIPE +writing to the helper rather than exiting because it was asked to. + +Let's make this one-off helper more like our other sample helpers, and +have it source the "dump" script. That will read stdin, fixing the +SIGPIPE problem. But it will also write what it sees to stderr. We can +make the test more robust by checking that output, which confirms that +we do run the quit helper, don't run any other helpers, and exit for the +reason we expected. + +Signed-off-by: Jeff King +Signed-off-by: Jonathan Nieder +(cherry picked from commit a88dbd2f8c7fd8c1e2f63483da03bd6928e8791f) +Signed-off-by: Jonathan Nieder +--- + t/t0300-credentials.sh | 16 +++++++++++++--- + 1 file changed, 13 insertions(+), 3 deletions(-) + +diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh +index b9c0f1f279e..a1d4d596b90 100755 +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -22,6 +22,11 @@ test_expect_success 'setup helper scripts' ' + exit 0 + EOF + ++ write_script git-credential-quit <<-\EOF && ++ . ./dump ++ echo quit=1 ++ EOF ++ + write_script git-credential-verbatim <<-\EOF && + user=$1; shift + pass=$1; shift +@@ -291,10 +296,15 @@ test_expect_success 'http paths can be part of context' ' + + test_expect_success 'helpers can abort the process' ' + test_must_fail git \ +- -c credential.helper="!f() { echo quit=1; }; f" \ ++ -c credential.helper=quit \ + -c credential.helper="verbatim foo bar" \ +- credential fill >stdout && +- test_must_be_empty stdout ++ credential fill >stdout 2>stderr && ++ test_must_be_empty stdout && ++ cat >expect <<-\EOF && ++ quit: get ++ fatal: credential helper '\''quit'\'' told us to quit ++ EOF ++ test_i18ncmp expect stderr + ' + + test_expect_success 'empty helper spec resets helper list' ' +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0049-t0300-use-more-realistic-inputs.diff git-2.20.1/debian/patches/0049-t0300-use-more-realistic-inputs.diff --- git-2.20.1/debian/patches/0049-t0300-use-more-realistic-inputs.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0049-t0300-use-more-realistic-inputs.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,289 @@ +From c70d5e5c90fe319e81b382788a8bbfa13322c2ad Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sat, 18 Apr 2020 20:47:30 -0700 +Subject: t0300: use more realistic inputs + +Many of the tests in t0300 give partial inputs to git-credential, +omitting a protocol or hostname. We're checking only high-level things +like whether and how helpers are invoked at all, and we don't care about +specific hosts. However, in preparation for tightening up the rules +about when we're willing to run a helper, let's start using input that's +a bit more realistic: pretend as if http://example.com is being +examined. + +This shouldn't change the point of any of the tests, but do note we have +to adjust the expected output to accommodate this (filling a credential +will repeat back the protocol/host fields to stdout, and the helper +debug messages and askpass prompt will change on stderr). + +Signed-off-by: Jeff King +Reviewed-by: Taylor Blau +Signed-off-by: Jonathan Nieder +(cherry picked from commit 73aafe9bc27585554181c58871a25e6d0f58a3dc) +Signed-off-by: Jonathan Nieder +--- + t/t0300-credentials.sh | 89 ++++++++++++++++++++++++++++++++++++++++-- + 1 file changed, 85 insertions(+), 4 deletions(-) + +diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh +index a1d4d596b90..fb63ab85834 100755 +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -40,43 +40,71 @@ test_expect_success 'setup helper scripts' ' + + test_expect_success 'credential_fill invokes helper' ' + check fill "verbatim foo bar" <<-\EOF ++ protocol=http ++ host=example.com + -- ++ protocol=http ++ host=example.com + username=foo + password=bar + -- + verbatim: get ++ verbatim: protocol=http ++ verbatim: host=example.com + EOF + ' + + test_expect_success 'credential_fill invokes multiple helpers' ' + check fill useless "verbatim foo bar" <<-\EOF ++ protocol=http ++ host=example.com + -- ++ protocol=http ++ host=example.com + username=foo + password=bar + -- + useless: get ++ useless: protocol=http ++ useless: host=example.com + verbatim: get ++ verbatim: protocol=http ++ verbatim: host=example.com + EOF + ' + + test_expect_success 'credential_fill stops when we get a full response' ' + check fill "verbatim one two" "verbatim three four" <<-\EOF ++ protocol=http ++ host=example.com + -- ++ protocol=http ++ host=example.com + username=one + password=two + -- + verbatim: get ++ verbatim: protocol=http ++ verbatim: host=example.com + EOF + ' + + test_expect_success 'credential_fill continues through partial response' ' + check fill "verbatim one \"\"" "verbatim two three" <<-\EOF ++ protocol=http ++ host=example.com + -- ++ protocol=http ++ host=example.com + username=two + password=three + -- + verbatim: get ++ verbatim: protocol=http ++ verbatim: host=example.com + verbatim: get ++ verbatim: protocol=http ++ verbatim: host=example.com + verbatim: username=one + EOF + ' +@@ -102,14 +130,20 @@ test_expect_success 'credential_fill passes along metadata' ' + + test_expect_success 'credential_approve calls all helpers' ' + check approve useless "verbatim one two" <<-\EOF ++ protocol=http ++ host=example.com + username=foo + password=bar + -- + -- + useless: store ++ useless: protocol=http ++ useless: host=example.com + useless: username=foo + useless: password=bar + verbatim: store ++ verbatim: protocol=http ++ verbatim: host=example.com + verbatim: username=foo + verbatim: password=bar + EOF +@@ -117,6 +151,8 @@ test_expect_success 'credential_approve calls all helpers' ' + + test_expect_success 'do not bother storing password-less credential' ' + check approve useless <<-\EOF ++ protocol=http ++ host=example.com + username=foo + -- + -- +@@ -126,14 +162,20 @@ test_expect_success 'do not bother storing password-less credential' ' + + test_expect_success 'credential_reject calls all helpers' ' + check reject useless "verbatim one two" <<-\EOF ++ protocol=http ++ host=example.com + username=foo + password=bar + -- + -- + useless: erase ++ useless: protocol=http ++ useless: host=example.com + useless: username=foo + useless: password=bar + verbatim: erase ++ verbatim: protocol=http ++ verbatim: host=example.com + verbatim: username=foo + verbatim: password=bar + EOF +@@ -141,33 +183,49 @@ test_expect_success 'credential_reject calls all helpers' ' + + test_expect_success 'usernames can be preserved' ' + check fill "verbatim \"\" three" <<-\EOF ++ protocol=http ++ host=example.com + username=one + -- ++ protocol=http ++ host=example.com + username=one + password=three + -- + verbatim: get ++ verbatim: protocol=http ++ verbatim: host=example.com + verbatim: username=one + EOF + ' + + test_expect_success 'usernames can be overridden' ' + check fill "verbatim two three" <<-\EOF ++ protocol=http ++ host=example.com + username=one + -- ++ protocol=http ++ host=example.com + username=two + password=three + -- + verbatim: get ++ verbatim: protocol=http ++ verbatim: host=example.com + verbatim: username=one + EOF + ' + + test_expect_success 'do not bother completing already-full credential' ' + check fill "verbatim three four" <<-\EOF ++ protocol=http ++ host=example.com + username=one + password=two + -- ++ protocol=http ++ host=example.com + username=one + password=two + -- +@@ -179,23 +237,31 @@ test_expect_success 'do not bother completing already-full credential' ' + # askpass helper is run, we know the internal getpass is working. + test_expect_success 'empty helper list falls back to internal getpass' ' + check fill <<-\EOF ++ protocol=http ++ host=example.com + -- ++ protocol=http ++ host=example.com + username=askpass-username + password=askpass-password + -- +- askpass: Username: +- askpass: Password: ++ askpass: Username for '\''http://example.com'\'': ++ askpass: Password for '\''http://askpass-username@example.com'\'': + EOF + ' + + test_expect_success 'internal getpass does not ask for known username' ' + check fill <<-\EOF ++ protocol=http ++ host=example.com + username=foo + -- ++ protocol=http ++ host=example.com + username=foo + password=askpass-password + -- +- askpass: Password: ++ askpass: Password for '\''http://foo@example.com'\'': + EOF + ' + +@@ -207,7 +273,11 @@ HELPER="!f() { + test_expect_success 'respect configured credentials' ' + test_config credential.helper "$HELPER" && + check fill <<-\EOF ++ protocol=http ++ host=example.com + -- ++ protocol=http ++ host=example.com + username=foo + password=bar + -- +@@ -298,10 +368,15 @@ test_expect_success 'helpers can abort the process' ' + test_must_fail git \ + -c credential.helper=quit \ + -c credential.helper="verbatim foo bar" \ +- credential fill >stdout 2>stderr && ++ credential fill >stdout 2>stderr <<-\EOF && ++ protocol=http ++ host=example.com ++ EOF + test_must_be_empty stdout && + cat >expect <<-\EOF && + quit: get ++ quit: protocol=http ++ quit: host=example.com + fatal: credential helper '\''quit'\'' told us to quit + EOF + test_i18ncmp expect stderr +@@ -310,11 +385,17 @@ test_expect_success 'helpers can abort the process' ' + test_expect_success 'empty helper spec resets helper list' ' + test_config credential.helper "verbatim file file" && + check fill "" "verbatim cmdline cmdline" <<-\EOF ++ protocol=http ++ host=example.com + -- ++ protocol=http ++ host=example.com + username=cmdline + password=cmdline + -- + verbatim: get ++ verbatim: protocol=http ++ verbatim: host=example.com + EOF + ' + +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0050-credential-parse-URL-without-host-as-empty-host-not-u.diff git-2.20.1/debian/patches/0050-credential-parse-URL-without-host-as-empty-host-not-u.diff --- git-2.20.1/debian/patches/0050-credential-parse-URL-without-host-as-empty-host-not-u.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0050-credential-parse-URL-without-host-as-empty-host-not-u.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,94 @@ +From 315d28801cad6f0de64f60d0875b068c0165a975 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sat, 18 Apr 2020 20:48:05 -0700 +Subject: credential: parse URL without host as empty host, not unset + +We may feed a URL like "cert:///path/to/cert.pem" into the credential +machinery to get the key for a client-side certificate. That +credential has no hostname field, which is about to be disallowed (to +avoid confusion with protocols where a helper _would_ expect a +hostname). + +This means as of the next patch, credential helpers won't work for +unlocking certs. Let's fix that by doing two things: + + - when we parse a url with an empty host, set the host field to the + empty string (asking only to match stored entries with an empty + host) rather than NULL (asking to match _any_ host). + + - when we build a cert:// credential by hand, similarly assign an + empty string + +It's the latter that is more likely to impact real users in practice, +since it's what's used for http connections. But we don't have good +infrastructure to test it. + +The url-parsing version will help anybody using git-credential in a +script, and is easy to test. + +Signed-off-by: Jeff King +Reviewed-by: Taylor Blau +Signed-off-by: Jonathan Nieder +(cherry picked from commit 24036686c4af84c9e84e486ef3debab6e6d8e6b5) +Signed-off-by: Jonathan Nieder +--- + credential.c | 3 +-- + http.c | 1 + + t/t0300-credentials.sh | 17 +++++++++++++++++ + 3 files changed, 19 insertions(+), 2 deletions(-) + +diff --git a/credential.c b/credential.c +index 24823829f66..f2413cec145 100644 +--- a/credential.c ++++ b/credential.c +@@ -376,8 +376,7 @@ int credential_from_url_gently(struct credential *c, const char *url, + + if (proto_end - url > 0) + c->protocol = xmemdupz(url, proto_end - url); +- if (slash - host > 0) +- c->host = url_decode_mem(host, slash - host); ++ c->host = url_decode_mem(host, slash - host); + /* Trim leading and trailing slashes from path */ + while (*slash == '/') + slash++; +diff --git a/http.c b/http.c +index eacc2a75ef2..4abe11c1253 100644 +--- a/http.c ++++ b/http.c +@@ -554,6 +554,7 @@ static int has_cert_password(void) + return 0; + if (!cert_auth.password) { + cert_auth.protocol = xstrdup("cert"); ++ cert_auth.host = xstrdup(""); + cert_auth.username = xstrdup(""); + cert_auth.path = xstrdup(ssl_cert); + credential_fill(&cert_auth); +diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh +index fb63ab85834..771fe6b8ccb 100755 +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -413,4 +413,21 @@ test_expect_success 'url parser ignores embedded newlines' ' + EOF + ' + ++test_expect_success 'host-less URLs are parsed as empty host' ' ++ check fill "verbatim foo bar" <<-\EOF ++ url=cert:///path/to/cert.pem ++ -- ++ protocol=cert ++ host= ++ path=path/to/cert.pem ++ username=foo ++ password=bar ++ -- ++ verbatim: get ++ verbatim: protocol=cert ++ verbatim: host= ++ verbatim: path=path/to/cert.pem ++ EOF ++' ++ + test_done +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0051-credential-refuse-to-operate-when-missing-host-or-pro.diff git-2.20.1/debian/patches/0051-credential-refuse-to-operate-when-missing-host-or-pro.diff --- git-2.20.1/debian/patches/0051-credential-refuse-to-operate-when-missing-host-or-pro.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0051-credential-refuse-to-operate-when-missing-host-or-pro.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,170 @@ +From 97e3f93c52bfef9d7a52d25ceedd12e48decd8e5 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sat, 18 Apr 2020 20:50:48 -0700 +Subject: credential: refuse to operate when missing host or protocol + +The credential helper protocol was designed to be very flexible: the +fields it takes as input are treated as a pattern, and any missing +fields are taken as wildcards. This allows unusual things like: + + echo protocol=https | git credential reject + +to delete all stored https credentials (assuming the helpers themselves +treat the input that way). But when helpers are invoked automatically by +Git, this flexibility works against us. If for whatever reason we don't +have a "host" field, then we'd match _any_ host. When you're filling a +credential to send to a remote server, this is almost certainly not what +you want. + +Prevent this at the layer that writes to the credential helper. Add a +check to the credential API that the host and protocol are always passed +in, and add an assertion to the credential_write function that speaks +credential helper protocol to be doubly sure. + +There are a few ways this can be triggered in practice: + + - the "git credential" command passes along arbitrary credential + parameters it reads from stdin. + + - until the previous patch, when the host field of a URL is empty, we + would leave it unset (rather than setting it to the empty string) + + - a URL like "example.com/foo.git" is treated by curl as if "http://" + was present, but our parser sees it as a non-URL and leaves all + fields unset + + - the recent fix for URLs with embedded newlines blanks the URL but + otherwise continues. Rather than having the desired effect of + looking up no credential at all, many helpers will return _any_ + credential + +Our earlier test for an embedded newline didn't catch this because it +only checked that the credential was cleared, but didn't configure an +actual helper. Configuring the "verbatim" helper in the test would show +that it is invoked (it's obviously a silly helper which doesn't look at +its input, but the point is that it shouldn't be run at all). Since +we're switching this case to die(), we don't need to bother with a +helper. We can see the new behavior just by checking that the operation +fails. + +We'll add new tests covering partial input as well (these can be +triggered through various means with url-parsing, but it's simpler to +just check them directly, as we know we are covered even if the url +parser changes behavior in the future). + +[jn: changed to die() instead of logging and showing a manual + username/password prompt] + +Reported-by: Carlo Arenas +Signed-off-by: Jeff King +Signed-off-by: Jonathan Nieder +(cherry picked from commit 8ba8ed568e2a3b75ee84c49ddffb026fde1a0a91) +Signed-off-by: Jonathan Nieder +--- + credential.c | 20 ++++++++++++++------ + t/t0300-credentials.sh | 34 ++++++++++++++++++++++++++-------- + 2 files changed, 40 insertions(+), 14 deletions(-) + +diff --git a/credential.c b/credential.c +index f2413cec145..e08ed84ff48 100644 +--- a/credential.c ++++ b/credential.c +@@ -89,6 +89,11 @@ static int proto_is_http(const char *s) + + static void credential_apply_config(struct credential *c) + { ++ if (!c->host) ++ die(_("refusing to work with credential missing host field")); ++ if (!c->protocol) ++ die(_("refusing to work with credential missing protocol field")); ++ + if (c->configured) + return; + git_config(credential_config_callback, c); +@@ -191,8 +196,11 @@ int credential_read(struct credential *c, FILE *fp) + return 0; + } + +-static void credential_write_item(FILE *fp, const char *key, const char *value) ++static void credential_write_item(FILE *fp, const char *key, const char *value, ++ int required) + { ++ if (!value && required) ++ BUG("credential value for %s is missing", key); + if (!value) + return; + if (strchr(value, '\n')) +@@ -202,11 +210,11 @@ static void credential_write_item(FILE *fp, const char *key, const char *value) + + void credential_write(const struct credential *c, FILE *fp) + { +- credential_write_item(fp, "protocol", c->protocol); +- credential_write_item(fp, "host", c->host); +- credential_write_item(fp, "path", c->path); +- credential_write_item(fp, "username", c->username); +- credential_write_item(fp, "password", c->password); ++ credential_write_item(fp, "protocol", c->protocol, 1); ++ credential_write_item(fp, "host", c->host, 1); ++ credential_write_item(fp, "path", c->path, 0); ++ credential_write_item(fp, "username", c->username, 0); ++ credential_write_item(fp, "password", c->password, 0); + } + + static int run_credential_helper(struct credential *c, +diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh +index 771fe6b8ccb..acb2b243208 100755 +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -399,18 +399,16 @@ test_expect_success 'empty helper spec resets helper list' ' + EOF + ' + +-test_expect_success 'url parser ignores embedded newlines' ' +- check fill <<-EOF ++test_expect_success 'url parser rejects embedded newlines' ' ++ test_must_fail git credential fill 2>stderr <<-\EOF && + url=https://one.example.com?%0ahost=two.example.com/ +- -- +- username=askpass-username +- password=askpass-password +- -- ++ EOF ++ cat >expect <<-\EOF && + warning: url contains a newline in its host component: https://one.example.com?%0ahost=two.example.com/ + warning: skipping credential lookup for url: https://one.example.com?%0ahost=two.example.com/ +- askpass: Username: +- askpass: Password: ++ fatal: refusing to work with credential missing host field + EOF ++ test_i18ncmp expect stderr + ' + + test_expect_success 'host-less URLs are parsed as empty host' ' +@@ -430,4 +428,24 @@ test_expect_success 'host-less URLs are parsed as empty host' ' + EOF + ' + ++test_expect_success 'credential system refuses to work with missing host' ' ++ test_must_fail git credential fill 2>stderr <<-\EOF && ++ protocol=http ++ EOF ++ cat >expect <<-\EOF && ++ fatal: refusing to work with credential missing host field ++ EOF ++ test_i18ncmp expect stderr ++' ++ ++test_expect_success 'credential system refuses to work with missing protocol' ' ++ test_must_fail git credential fill 2>stderr <<-\EOF && ++ host=example.com ++ EOF ++ cat >expect <<-\EOF && ++ fatal: refusing to work with credential missing protocol field ++ EOF ++ test_i18ncmp expect stderr ++' ++ + test_done +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0052-fsck-convert-gitmodules-url-to-URL-passed-to-curl.diff git-2.20.1/debian/patches/0052-fsck-convert-gitmodules-url-to-URL-passed-to-curl.diff --- git-2.20.1/debian/patches/0052-fsck-convert-gitmodules-url-to-URL-passed-to-curl.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0052-fsck-convert-gitmodules-url-to-URL-passed-to-curl.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,209 @@ +From fe6f0301d2cf6606214b23f1021031dba3680712 Mon Sep 17 00:00:00 2001 +From: Jonathan Nieder +Date: Sat, 18 Apr 2020 20:52:34 -0700 +Subject: fsck: convert gitmodules url to URL passed to curl + +In 07259e74ec1 (fsck: detect gitmodules URLs with embedded newlines, +2020-03-11), git fsck learned to check whether URLs in .gitmodules could +be understood by the credential machinery when they are handled by +git-remote-curl. + +However, the check is overbroad: it checks all URLs instead of only +URLs that would be passed to git-remote-curl. In principle a git:// or +file:/// URL does not need to follow the same conventions as an http:// +URL; in particular, git:// and file:// protocols are not succeptible to +issues in the credential API because they do not support attaching +credentials. + +In the HTTP case, the URL in .gitmodules does not always match the URL +that would be passed to git-remote-curl and the credential machinery: +Git's URL syntax allows specifying a remote helper followed by a "::" +delimiter and a URL to be passed to it, so that + + git ls-remote http::https://example.com/repo.git + +invokes git-remote-http with https://example.com/repo.git as its URL +argument. With today's checks, that distinction does not make a +difference, but for a check we are about to introduce (for empty URL +schemes) it will matter. + +.gitmodules files also support relative URLs. To ensure coverage for the +https based embedded-newline attack, urldecode and check them directly +for embedded newlines. + +Helped-by: Jeff King +Signed-off-by: Jonathan Nieder +Reviewed-by: Jeff King +(cherry picked from commit a2b26ffb1a81aa23dd14453f4db05d8fe24ee7cc) +Signed-off-by: Jonathan Nieder +--- + fsck.c | 94 +++++++++++++++++++++++++++++++++-- + t/t7416-submodule-dash-url.sh | 29 +++++++++++ + 2 files changed, 118 insertions(+), 5 deletions(-) + +diff --git a/fsck.c b/fsck.c +index faac610a355..046e2f2a3d8 100644 +--- a/fsck.c ++++ b/fsck.c +@@ -9,6 +9,7 @@ + #include "tag.h" + #include "fsck.h" + #include "refs.h" ++#include "url.h" + #include "utf8.h" + #include "decorate.h" + #include "oidset.h" +@@ -983,17 +984,100 @@ static int fsck_tag(struct tag *tag, const char *data, + return fsck_tag_buffer(tag, data, size, options); + } + ++/* ++ * Like builtin/submodule--helper.c's starts_with_dot_slash, but without ++ * relying on the platform-dependent is_dir_sep helper. ++ * ++ * This is for use in checking whether a submodule URL is interpreted as ++ * relative to the current directory on any platform, since \ is a ++ * directory separator on Windows but not on other platforms. ++ */ ++static int starts_with_dot_slash(const char *str) ++{ ++ return str[0] == '.' && (str[1] == '/' || str[1] == '\\'); ++} ++ ++/* ++ * Like starts_with_dot_slash, this is a variant of submodule--helper's ++ * helper of the same name with the twist that it accepts backslash as a ++ * directory separator even on non-Windows platforms. ++ */ ++static int starts_with_dot_dot_slash(const char *str) ++{ ++ return str[0] == '.' && starts_with_dot_slash(str + 1); ++} ++ ++static int submodule_url_is_relative(const char *url) ++{ ++ return starts_with_dot_slash(url) || starts_with_dot_dot_slash(url); ++} ++ ++/* ++ * 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. ++ * ++ * Otherwise, returns 0 and leaves "out" untouched. ++ * ++ * Examples: ++ * http::https://example.com/repo.git -> 1, https://example.com/repo.git ++ * https://example.com/repo.git -> 1, https://example.com/repo.git ++ * git://example.com/repo.git -> 0 ++ * ++ * This is for use in checking for previously exploitable bugs that ++ * required a submodule URL to be passed to git-remote-curl. ++ */ ++static int url_to_curl_url(const char *url, const char **out) ++{ ++ /* ++ * We don't need to check for case-aliases, "http.exe", and so ++ * on because in the default configuration, is_transport_allowed ++ * prevents URLs with those schemes from being cloned ++ * automatically. ++ */ ++ if (skip_prefix(url, "http::", out) || ++ skip_prefix(url, "https::", out) || ++ skip_prefix(url, "ftp::", out) || ++ skip_prefix(url, "ftps::", out)) ++ return 1; ++ if (starts_with(url, "http://") || ++ starts_with(url, "https://") || ++ starts_with(url, "ftp://") || ++ starts_with(url, "ftps://")) { ++ *out = url; ++ return 1; ++ } ++ return 0; ++} ++ + static int check_submodule_url(const char *url) + { +- struct credential c = CREDENTIAL_INIT; +- int ret; ++ const char *curl_url; + + if (looks_like_command_line_option(url)) + return -1; + +- ret = credential_from_url_gently(&c, url, 1); +- credential_clear(&c); +- return ret; ++ if (submodule_url_is_relative(url)) { ++ /* ++ * 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'); ++ free(decoded); ++ if (has_nl) ++ 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); ++ credential_clear(&c); ++ return ret; ++ } ++ ++ return 0; + } + + struct fsck_gitmodules_data { +diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh +index 41431b1ac38..afdd2553d91 100755 +--- a/t/t7416-submodule-dash-url.sh ++++ b/t/t7416-submodule-dash-url.sh +@@ -60,6 +60,20 @@ test_expect_success 'trailing backslash is handled correctly' ' + test_i18ngrep ! "unknown option" err + ' + ++test_expect_success 'fsck permits embedded newline with unrecognized scheme' ' ++ git checkout --orphan newscheme && ++ cat >.gitmodules <<-\EOF && ++ [submodule "foo"] ++ url = "data://acjbkd%0akajfdickajkd" ++ EOF ++ git add .gitmodules && ++ git commit -m "gitmodules with unrecognized scheme" && ++ test_when_finished "rm -rf dst" && ++ git init --bare dst && ++ git -C dst config transfer.fsckObjects true && ++ git push dst HEAD ++' ++ + test_expect_success 'fsck rejects embedded newline in url' ' + # create an orphan branch to avoid existing .gitmodules objects + git checkout --orphan newline && +@@ -76,4 +90,19 @@ test_expect_success 'fsck rejects embedded newline in url' ' + grep gitmodulesUrl err + ' + ++test_expect_success 'fsck rejects embedded newline in relative url' ' ++ git checkout --orphan relative-newline && ++ cat >.gitmodules <<-\EOF && ++ [submodule "foo"] ++ url = "./%0ahost=two.example.com/foo.git" ++ EOF ++ git add .gitmodules && ++ git commit -m "relative url with newline" && ++ 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_done +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0053-credential-die-when-parsing-invalid-urls.diff git-2.20.1/debian/patches/0053-credential-die-when-parsing-invalid-urls.diff --- git-2.20.1/debian/patches/0053-credential-die-when-parsing-invalid-urls.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0053-credential-die-when-parsing-invalid-urls.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,82 @@ +From 2062d1fb629c87e2b2f5921c8276026b33978799 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sat, 18 Apr 2020 20:53:09 -0700 +Subject: credential: die() when parsing invalid urls + +When we try to initialize credential loading by URL and find that the +URL is invalid, we set all fields to NULL in order to avoid acting on +malicious input. Later when we request credentials, we diagonse the +erroneous input: + + fatal: refusing to work with credential missing host field + +This is problematic in two ways: + +- The message doesn't tell the user *why* we are missing the host + field, so they can't tell from this message alone how to recover. + There can be intervening messages after the original warning of + bad input, so the user may not have the context to put two and two + together. + +- The error only occurs when we actually need to get a credential. If + the URL permits anonymous access, the only encouragement the user gets + to correct their bogus URL is a quiet warning. + + This is inconsistent with the check we perform in fsck, where any use + of such a URL as a submodule is an error. + +When we see such a bogus URL, let's not try to be nice and continue +without helpers. Instead, die() immediately. This is simpler and +obviously safe. And there's very little chance of disrupting a normal +workflow. + +It's _possible_ that somebody has a legitimate URL with a raw newline in +it. It already wouldn't work with credential helpers, so this patch +steps that up from an inconvenience to "we will refuse to work with it +at all". If such a case does exist, we should figure out a way to work +with it (especially if the newline is only in the path component, which +we normally don't even pass to helpers). But until we see a real report, +we're better off being defensive. + +Reported-by: Carlo Arenas +Signed-off-by: Jeff King +Signed-off-by: Jonathan Nieder +(cherry picked from commit fe29a9b7b0236d3d45c254965580d6aff7fa8504) +Signed-off-by: Jonathan Nieder +--- + credential.c | 6 ++---- + t/t0300-credentials.sh | 3 +-- + 2 files changed, 3 insertions(+), 6 deletions(-) + +diff --git a/credential.c b/credential.c +index e08ed84ff48..22649d559f1 100644 +--- a/credential.c ++++ b/credential.c +@@ -408,8 +408,6 @@ int credential_from_url_gently(struct credential *c, const char *url, + + void credential_from_url(struct credential *c, const char *url) + { +- if (credential_from_url_gently(c, url, 0) < 0) { +- warning(_("skipping credential lookup for url: %s"), url); +- credential_clear(c); +- } ++ if (credential_from_url_gently(c, url, 0) < 0) ++ die(_("credential url cannot be parsed: %s"), url); + } +diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh +index acb2b243208..6d44e7e5ccd 100755 +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -405,8 +405,7 @@ test_expect_success 'url parser rejects embedded newlines' ' + EOF + cat >expect <<-\EOF && + warning: url contains a newline in its host component: https://one.example.com?%0ahost=two.example.com/ +- warning: skipping credential lookup for url: https://one.example.com?%0ahost=two.example.com/ +- fatal: refusing to work with credential missing host field ++ fatal: credential url cannot be parsed: https://one.example.com?%0ahost=two.example.com/ + EOF + test_i18ncmp expect stderr + ' +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0054-credential-treat-URL-without-scheme-as-invalid.diff git-2.20.1/debian/patches/0054-credential-treat-URL-without-scheme-as-invalid.diff --- git-2.20.1/debian/patches/0054-credential-treat-URL-without-scheme-as-invalid.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0054-credential-treat-URL-without-scheme-as-invalid.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,204 @@ +From 756cedc3a075b1974140d85ee8fda2a19af6f250 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 +(cherry picked from commit c44088ecc4b0722636e0a305f9608d3047197282) +Signed-off-by: Jonathan Nieder +--- + credential.c | 7 ++++-- + fsck.c | 47 +++++++++++++++++++++++++++++++++-- + t/t5550-http-fetch-dumb.sh | 7 ++---- + t/t7416-submodule-dash-url.sh | 32 ++++++++++++++++++++++++ + 4 files changed, 84 insertions(+), 9 deletions(-) + +diff --git a/credential.c b/credential.c +index 22649d559f1..1e1aed59942 100644 +--- a/credential.c ++++ b/credential.c +@@ -360,8 +360,11 @@ int credential_from_url_gently(struct credential *c, const char *url, + * (3) proto://:@/... + */ + 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, ':'); +diff --git a/fsck.c b/fsck.c +index 046e2f2a3d8..c6ebd7d9466 100644 +--- a/fsck.c ++++ b/fsck.c +@@ -1012,6 +1012,34 @@ 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. + * +@@ -1059,15 +1087,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 6d7d88ccc90..7cc49a6d3d7 100755 +--- a/t/t5550-http-fetch-dumb.sh ++++ b/t/t5550-http-fetch-dumb.sh +@@ -321,11 +321,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 afdd2553d91..249dc3d1d46 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 && +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0055-credential-treat-URL-with-empty-scheme-as-invalid.diff git-2.20.1/debian/patches/0055-credential-treat-URL-with-empty-scheme-as-invalid.diff --- git-2.20.1/debian/patches/0055-credential-treat-URL-with-empty-scheme-as-invalid.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0055-credential-treat-URL-with-empty-scheme-as-invalid.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,112 @@ +From b46a27e2fefee272019375183eed56fc64783a5f 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 +(cherry picked from commit e7fab62b736cca3416660636e46f0be8386a5030) +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(-) + +diff --git a/credential.c b/credential.c +index 1e1aed59942..cf11cc98f4f 100644 +--- a/credential.c ++++ b/credential.c +@@ -360,7 +360,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; +@@ -385,8 +385,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 7cc49a6d3d7..bf9ca5a8ecb 100755 +--- a/t/t5550-http-fetch-dumb.sh ++++ b/t/t5550-http-fetch-dumb.sh +@@ -325,6 +325,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 249dc3d1d46..9309040373c 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 && +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0056-fsck-reject-URL-with-empty-host-in-.gitmodules.diff git-2.20.1/debian/patches/0056-fsck-reject-URL-with-empty-host-in-.gitmodules.diff --- git-2.20.1/debian/patches/0056-fsck-reject-URL-with-empty-host-in-.gitmodules.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0056-fsck-reject-URL-with-empty-host-in-.gitmodules.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,112 @@ +From 75eceba8449bcff5a155c31855e19d678996e904 Mon Sep 17 00:00:00 2001 +From: Jonathan Nieder +Date: Sat, 18 Apr 2020 20:57:22 -0700 +Subject: fsck: reject URL with empty host in .gitmodules + +Git's URL parser interprets + + https:///example.com/repo.git + +to have no host and a path of "example.com/repo.git". Curl, on the +other hand, internally redirects it to https://example.com/repo.git. As +a result, until "credential: parse URL without host as empty host, not +unset", tricking a user into fetching from such a URL would cause Git to +send credentials for another host to example.com. + +Teach fsck to block and detect .gitmodules files using such a URL to +prevent sharing them with Git versions that are not yet protected. + +A relative URL in a .gitmodules file could also be used to trigger this. +The relative URL resolver used for .gitmodules does not normalize +sequences of slashes and can follow ".." components out of the path part +and to 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, +redundant extra slashes in .gitmodules are rare, so we can catch this by +detecting one after a leading sequence of "./" and "../" components. + +Helped-by: Jeff King +Signed-off-by: Jonathan Nieder +Reviewed-by: Jeff King +(cherry picked from commit 1a3609e402a062ef7b11f197fe96c28cabca132c) +Signed-off-by: Jonathan Nieder +--- + fsck.c | 10 +++++++--- + t/t7416-submodule-dash-url.sh | 32 ++++++++++++++++++++++++++++++++ + 2 files changed, 39 insertions(+), 3 deletions(-) + +diff --git a/fsck.c b/fsck.c +index c6ebd7d9466..c4a9fe624a9 100644 +--- a/fsck.c ++++ b/fsck.c +@@ -1105,17 +1105,21 @@ static int check_submodule_url(const char *url) + /* + * 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 ++ * URLs like https::example.com/submodule.git and ++ * https:///example.com/submodule.git that were + * susceptible to CVE-2020-11008. + */ + if (count_leading_dotdots(url, &next) > 0 && +- *next == ':') ++ (*next == ':' || *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); ++ int ret = 0; ++ if (credential_from_url_gently(&c, curl_url, 1) || ++ !*c.host) ++ ret = -1; + credential_clear(&c); + return ret; + } +diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh +index 9309040373c..eec96e0ba9e 100755 +--- a/t/t7416-submodule-dash-url.sh ++++ b/t/t7416-submodule-dash-url.sh +@@ -124,6 +124,38 @@ test_expect_success 'fsck rejects relative URL resolving to empty scheme' ' + grep gitmodulesUrl err + ' + ++test_expect_success 'fsck rejects empty hostname' ' ++ git checkout --orphan empty-host && ++ cat >.gitmodules <<-\EOF && ++ [submodule "foo"] ++ url = http:///one.example.com/foo.git ++ EOF ++ git add .gitmodules && ++ test_tick && ++ git commit -m "gitmodules with extra slashes" && ++ 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 that produced empty hostname' ' ++ git checkout --orphan messy-relative && ++ cat >.gitmodules <<-\EOF && ++ [submodule "foo"] ++ url = ../../..//one.example.com/foo.git ++ EOF ++ git add .gitmodules && ++ test_tick && ++ git commit -m "gitmodules abusing relative_path" && ++ 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 && +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0057-Git-2.17.5.diff git-2.20.1/debian/patches/0057-Git-2.17.5.diff --- git-2.20.1/debian/patches/0057-Git-2.17.5.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0057-Git-2.17.5.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,45 @@ +From ee301385d0ec4625afd825f1d6f667999753d8b3 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sun, 19 Apr 2020 02:34:55 -0400 +Subject: Git 2.17.5 + +Signed-off-by: Jeff King +Signed-off-by: Jonathan Nieder +(cherry picked from commit df5be6dc3fd18c294ec93a9af0321334e3f92c9c) +Signed-off-by: Jonathan Nieder +--- + Documentation/RelNotes/2.17.5.txt | 22 ++++++++++++++++++++++ + 1 file changed, 22 insertions(+) + create mode 100644 Documentation/RelNotes/2.17.5.txt + +diff --git a/Documentation/RelNotes/2.17.5.txt b/Documentation/RelNotes/2.17.5.txt +new file mode 100644 +index 00000000000..2abb821a739 +--- /dev/null ++++ b/Documentation/RelNotes/2.17.5.txt +@@ -0,0 +1,22 @@ ++Git v2.17.5 Release Notes ++========================= ++ ++This release is to address a security issue: CVE-2020-11008 ++ ++Fixes since v2.17.4 ++------------------- ++ ++ * With a crafted URL that contains a newline or empty host, or lacks ++ a scheme, the credential helper machinery can be fooled into ++ providing credential information that is not appropriate for the ++ protocol in use and host being contacted. ++ ++ Unlike the vulnerability CVE-2020-5260 fixed in v2.17.4, the ++ credentials are not for a host of the attacker's choosing; instead, ++ they are for some unspecified host (based on how the configured ++ credential helper handles an absent "host" parameter). ++ ++ The attack has been made impossible by refusing to work with ++ under-specified credential patterns. ++ ++Credit for finding the vulnerability goes to Carlo Arenas. +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0058-Git-2.18.4.diff git-2.20.1/debian/patches/0058-Git-2.18.4.diff --- git-2.20.1/debian/patches/0058-Git-2.18.4.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0058-Git-2.18.4.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,29 @@ +From 1bb39b32d8837b8dfcf17e1fc131edcf9429fdb7 Mon Sep 17 00:00:00 2001 +From: Jonathan Nieder +Date: Sun, 19 Apr 2020 16:24:14 -0700 +Subject: Git 2.18.4 + +This merges up the security fix from v2.17.5. + +Signed-off-by: Jonathan Nieder +(cherry picked from commit ba6f0905fdb9e65c1ac5fbc79c9a4ef0b59b3e68) +Signed-off-by: Jonathan Nieder +--- + Documentation/RelNotes/2.18.4.txt | 5 +++++ + 1 file changed, 5 insertions(+) + create mode 100644 Documentation/RelNotes/2.18.4.txt + +diff --git a/Documentation/RelNotes/2.18.4.txt b/Documentation/RelNotes/2.18.4.txt +new file mode 100644 +index 00000000000..e8ef858a00a +--- /dev/null ++++ b/Documentation/RelNotes/2.18.4.txt +@@ -0,0 +1,5 @@ ++Git v2.18.4 Release Notes ++========================= ++ ++This release merges the security fix that appears in v2.17.5; see ++the release notes for that version for details. +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0059-Git-2.19.5.diff git-2.20.1/debian/patches/0059-Git-2.19.5.diff --- git-2.20.1/debian/patches/0059-Git-2.19.5.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0059-Git-2.19.5.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,29 @@ +From 5c02b7802c36cee447aaa72825d72514d6e23ec3 Mon Sep 17 00:00:00 2001 +From: Jonathan Nieder +Date: Sun, 19 Apr 2020 16:26:41 -0700 +Subject: Git 2.19.5 + +This merges up the security fix from v2.17.5. + +Signed-off-by: Jonathan Nieder +(cherry picked from commit 76b54ee9b9944ee70422ac24884f78769cf264f1) +Signed-off-by: Jonathan Nieder +--- + Documentation/RelNotes/2.19.5.txt | 5 +++++ + 1 file changed, 5 insertions(+) + create mode 100644 Documentation/RelNotes/2.19.5.txt + +diff --git a/Documentation/RelNotes/2.19.5.txt b/Documentation/RelNotes/2.19.5.txt +new file mode 100644 +index 00000000000..18a4dcbfd6a +--- /dev/null ++++ b/Documentation/RelNotes/2.19.5.txt +@@ -0,0 +1,5 @@ ++Git v2.19.5 Release Notes ++========================= ++ ++This release merges the security fix that appears in v2.17.5; see ++the release notes for that version for details. +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/0060-Git-2.20.4.diff git-2.20.1/debian/patches/0060-Git-2.20.4.diff --- git-2.20.1/debian/patches/0060-Git-2.20.4.diff 1970-01-01 00:00:00.000000000 +0000 +++ git-2.20.1/debian/patches/0060-Git-2.20.4.diff 2020-04-20 00:19:12.000000000 +0000 @@ -0,0 +1,43 @@ +From 01ed1c21ca44db1296812acaae55cb186897ae0d Mon Sep 17 00:00:00 2001 +From: Jonathan Nieder +Date: Sun, 19 Apr 2020 16:28:57 -0700 +Subject: Git 2.20.4 + +This merges up the security fix from v2.17.5. + +Signed-off-by: Jonathan Nieder +(cherry picked from commit 041bc65923e13313ca1b77681c3b2950b5e0a163) +Signed-off-by: Jonathan Nieder +--- + Documentation/RelNotes/2.20.4.txt | 5 +++++ + GIT-VERSION-GEN | 2 +- + 2 files changed, 6 insertions(+), 1 deletion(-) + create mode 100644 Documentation/RelNotes/2.20.4.txt + +diff --git a/Documentation/RelNotes/2.20.4.txt b/Documentation/RelNotes/2.20.4.txt +new file mode 100644 +index 00000000000..5a9e24e4709 +--- /dev/null ++++ b/Documentation/RelNotes/2.20.4.txt +@@ -0,0 +1,5 @@ ++Git v2.20.4 Release Notes ++========================= ++ ++This release merges the security fix that appears in v2.17.5; see ++the release notes for that version for details. +diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN +index 1482b4d75f4..de22ee7afad 100755 +--- a/GIT-VERSION-GEN ++++ b/GIT-VERSION-GEN +@@ -1,7 +1,7 @@ + #!/bin/sh + + GVF=GIT-VERSION-FILE +-DEF_VER=v2.20.3 ++DEF_VER=v2.20.4 + + LF=' + ' +-- +2.26.1.301.g55bc3eb7cb9 + diff -Nru git-2.20.1/debian/patches/series git-2.20.1/debian/patches/series --- git-2.20.1/debian/patches/series 2020-04-12 07:24:43.000000000 +0000 +++ git-2.20.1/debian/patches/series 2020-04-20 00:19:12.000000000 +0000 @@ -45,3 +45,16 @@ 0045-Git-2.18.3.diff 0046-Git-2.19.4.diff 0047-Git-2.20.3.diff +0048-t0300-make-quit-helper-more-realistic.diff +0049-t0300-use-more-realistic-inputs.diff +0050-credential-parse-URL-without-host-as-empty-host-not-u.diff +0051-credential-refuse-to-operate-when-missing-host-or-pro.diff +0052-fsck-convert-gitmodules-url-to-URL-passed-to-curl.diff +0053-credential-die-when-parsing-invalid-urls.diff +0054-credential-treat-URL-without-scheme-as-invalid.diff +0055-credential-treat-URL-with-empty-scheme-as-invalid.diff +0056-fsck-reject-URL-with-empty-host-in-.gitmodules.diff +0057-Git-2.17.5.diff +0058-Git-2.18.4.diff +0059-Git-2.19.5.diff +0060-Git-2.20.4.diff