diff --git a/CVE-2020-11008-1.patch b/CVE-2020-11008-1.patch new file mode 100644 index 0000000000000000000000000000000000000000..7b4ca7cd0045f3b235fa01de2adbce5b75e589aa --- /dev/null +++ b/CVE-2020-11008-1.patch @@ -0,0 +1,63 @@ +From a88dbd2f8c7fd8c1e2f63483da03bd6928e8791f Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sun, 19 Apr 2020 01:36:02 -0400 +Subject: [PATCH] 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 +--- + t/t0300-credentials.sh | 16 +++++++++++++--- + 1 file changed, 13 insertions(+), 3 deletions(-) + +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -22,6 +22,11 @@ test_expect_success 'setup helper script + 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 p + + 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' ' +-- +1.8.3.1 + diff --git a/CVE-2020-11008-2.patch b/CVE-2020-11008-2.patch new file mode 100644 index 0000000000000000000000000000000000000000..1ef8fa26f77aadd8d9deea9d1308869a34b6702a --- /dev/null +++ b/CVE-2020-11008-2.patch @@ -0,0 +1,285 @@ +From 73aafe9bc27585554181c58871a25e6d0f58a3dc Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sat, 18 Apr 2020 20:47:30 -0700 +Subject: [PATCH] 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 +--- + t/t0300-credentials.sh | 89 ++++++++++++++++++++++++++++++++++++++++-- + 1 file changed, 85 insertions(+), 4 deletions(-) + +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -40,43 +40,71 @@ test_expect_success 'setup helper script + + 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 pas + + 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 + + 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 stori + + 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 c + + 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 compl + # 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 t + 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 t + 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 + ' + +-- +1.8.3.1 + diff --git a/CVE-2020-11008-3.patch b/CVE-2020-11008-3.patch new file mode 100644 index 0000000000000000000000000000000000000000..ca65c3d8d75f73cddefc3354b773a7995bc4555d --- /dev/null +++ b/CVE-2020-11008-3.patch @@ -0,0 +1,86 @@ +From 24036686c4af84c9e84e486ef3debab6e6d8e6b5 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sat, 18 Apr 2020 20:48:05 -0700 +Subject: [PATCH] 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 +--- + credential.c | 3 +-- + http.c | 1 + + t/t0300-credentials.sh | 17 +++++++++++++++++ + 3 files changed, 19 insertions(+), 2 deletions(-) + +--- a/credential.c ++++ b/credential.c +@@ -376,8 +376,7 @@ int credential_from_url_gently(struct cr + + 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++; +--- 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); +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -413,4 +413,21 @@ test_expect_success 'url parser ignores + 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 +-- +1.8.3.1 + diff --git a/CVE-2020-11008-4.patch b/CVE-2020-11008-4.patch new file mode 100644 index 0000000000000000000000000000000000000000..93c70a0ddf30656f39766ec7f2d24d73a8b9b25a --- /dev/null +++ b/CVE-2020-11008-4.patch @@ -0,0 +1,164 @@ +From 8ba8ed568e2a3b75ee84c49ddffb026fde1a0a91 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sat, 18 Apr 2020 20:50:48 -0700 +Subject: [PATCH] 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 +--- + credential.c | 20 ++++++++++++++------ + t/t0300-credentials.sh | 34 ++++++++++++++++++++++++++-------- + 2 files changed, 40 insertions(+), 14 deletions(-) + +--- 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 + 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 * + + 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, +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -399,18 +399,16 @@ test_expect_success 'empty helper spec r + 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 + 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 +-- +1.8.3.1 + diff --git a/CVE-2020-11008-5.patch b/CVE-2020-11008-5.patch new file mode 100644 index 0000000000000000000000000000000000000000..a35b6985d2e217848f728001e4ce0a458528210a --- /dev/null +++ b/CVE-2020-11008-5.patch @@ -0,0 +1,205 @@ +From a2b26ffb1a81aa23dd14453f4db05d8fe24ee7cc Mon Sep 17 00:00:00 2001 +From: Jonathan Nieder +Date: Sat, 18 Apr 2020 20:52:34 -0700 +Subject: [PATCH] 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 +--- + fsck.c | 94 +++++++++++++++++++++++++++++++++-- + t/t7416-submodule-dash-url.sh | 29 +++++++++++ + 2 files changed, 118 insertions(+), 5 deletions(-) + +--- 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, con + 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 { +--- a/t/t7416-submodule-dash-url.sh ++++ b/t/t7416-submodule-dash-url.sh +@@ -60,6 +60,20 @@ test_expect_success 'trailing backslash + 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 && +@@ -72,6 +86,21 @@ test_expect_success 'fsck rejects embedd + 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 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 + ' +-- +1.8.3.1 + diff --git a/CVE-2020-11008-6.patch b/CVE-2020-11008-6.patch new file mode 100644 index 0000000000000000000000000000000000000000..b58fc676033a34c415cf49a95af221baf8705df6 --- /dev/null +++ b/CVE-2020-11008-6.patch @@ -0,0 +1,75 @@ +From fe29a9b7b0236d3d45c254965580d6aff7fa8504 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Sat, 18 Apr 2020 20:53:09 -0700 +Subject: [PATCH] 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 +--- + credential.c | 6 ++---- + t/t0300-credentials.sh | 3 +-- + 2 files changed, 3 insertions(+), 6 deletions(-) + +--- a/credential.c ++++ b/credential.c +@@ -408,8 +408,6 @@ int credential_from_url_gently(struct cr + + 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); + } +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -405,8 +405,7 @@ test_expect_success 'url parser rejects + 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 + ' +-- +1.8.3.1 diff --git a/CVE-2020-11008-7.patch b/CVE-2020-11008-7.patch new file mode 100644 index 0000000000000000000000000000000000000000..5fa360d9cb624d7f748fcefe38e495e6e88d0583 --- /dev/null +++ b/CVE-2020-11008-7.patch @@ -0,0 +1,194 @@ +From c44088ecc4b0722636e0a305f9608d3047197282 Mon Sep 17 00:00:00 2001 +From: Jonathan Nieder +Date: Sat, 18 Apr 2020 20:54:13 -0700 +Subject: [PATCH] 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 +--- + 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(-) + +--- a/credential.c ++++ b/credential.c +@@ -360,8 +360,11 @@ int credential_from_url_gently(struct cr + * (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, ':'); +--- a/fsck.c ++++ b/fsck.c +@@ -1013,6 +1013,34 @@ static int submodule_url_is_relative(con + } + + /* ++ * 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 +@@ -1059,15 +1087,30 @@ static int check_submodule_url(const cha + 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)) { +--- 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 + ' + + 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' ' +--- a/t/t7416-submodule-dash-url.sh ++++ b/t/t7416-submodule-dash-url.sh +@@ -60,6 +60,38 @@ test_expect_success 'trailing backslash + 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 && +-- +1.8.3.1 + diff --git a/CVE-2020-11008-8.patch b/CVE-2020-11008-8.patch new file mode 100644 index 0000000000000000000000000000000000000000..5d0b05ee345e86d81bb93d7de70d71185ec962b4 --- /dev/null +++ b/CVE-2020-11008-8.patch @@ -0,0 +1,104 @@ +From e7fab62b736cca3416660636e46f0be8386a5030 Mon Sep 17 00:00:00 2001 +From: Jonathan Nieder +Date: Sat, 18 Apr 2020 20:54:57 -0700 +Subject: [PATCH] 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(-) + +--- a/credential.c ++++ b/credential.c +@@ -360,7 +360,7 @@ int credential_from_url_gently(struct cr + * (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 cr + 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 == '/') +--- a/t/t5550-http-fetch-dumb.sh ++++ b/t/t5550-http-fetch-dumb.sh +@@ -325,6 +325,15 @@ test_expect_success 'remote-http complai + 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 && +--- a/t/t7416-submodule-dash-url.sh ++++ b/t/t7416-submodule-dash-url.sh +@@ -92,6 +92,38 @@ test_expect_success 'fsck rejects relati + 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 && +-- +1.8.3.1 + diff --git a/CVE-2020-11008-9.patch b/CVE-2020-11008-9.patch new file mode 100644 index 0000000000000000000000000000000000000000..05d3a2c02f5d3918bc60dce8ae8adc2c5204635a --- /dev/null +++ b/CVE-2020-11008-9.patch @@ -0,0 +1,106 @@ +From 1a3609e402a062ef7b11f197fe96c28cabca132c Mon Sep 17 00:00:00 2001 +From: Jonathan Nieder +Date: Sat, 18 Apr 2020 20:57:22 -0700 +Subject: [PATCH] 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 +--- + fsck.c | 10 +++++++--- + t/t7416-submodule-dash-url.sh | 32 ++++++++++++++++++++++++++++++++ + 2 files changed, 39 insertions(+), 3 deletions(-) + +--- a/fsck.c ++++ b/fsck.c +@@ -1105,17 +1105,21 @@ static int check_submodule_url(const cha + /* + * 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; + } +--- a/t/t7416-submodule-dash-url.sh ++++ b/t/t7416-submodule-dash-url.sh +@@ -124,6 +124,38 @@ test_expect_success 'fsck rejects relati + 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 && +-- +1.8.3.1 + diff --git a/credential-detect-unrepresentable-values-when-parsin.patch b/credential-detect-unrepresentable-values-when-parsin.patch new file mode 100644 index 0000000000000000000000000000000000000000..7f1102ae1177986a3f4af794587325ce4415e4c0 --- /dev/null +++ b/credential-detect-unrepresentable-values-when-parsin.patch @@ -0,0 +1,151 @@ +From c716fe4bd917e013bf376a678b3a924447777b2d Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Thu, 12 Mar 2020 01:31:11 -0400 +Subject: [PATCH] credential: detect unrepresentable values when parsing urls + +The credential protocol can't represent newlines in values, but URLs can +embed percent-encoded newlines in various components. A previous commit +taught the low-level writing routines to die() when encountering this, +but we can be a little friendlier to the user by detecting them earlier +and handling them gracefully. + +This patch teaches credential_from_url() to notice such components, +issue a warning, and blank the credential (which will generally result +in prompting the user for a username and password). We blank the whole +credential in this case. Another option would be to blank only the +invalid component. However, we're probably better off not feeding a +partially-parsed URL result to a credential helper. We don't know how a +given helper would handle it, so we're better off to err on the side of +matching nothing rather than something unexpected. + +The die() call in credential_write() is _probably_ impossible to reach +after this patch. Values should end up in credential structs only by URL +parsing (which is covered here), or by reading credential protocol input +(which by definition cannot read a newline into a value). But we should +definitely keep the low-level check, as it's our final and most accurate +line of defense against protocol injection attacks. Arguably it could +become a BUG(), but it probably doesn't matter much either way. + +Note that the public interface of credential_from_url() grows a little +more than we need here. We'll use the extra flexibility in a future +patch to help fsck catch these cases. +--- + credential.c | 36 ++++++++++++++++++++++++++++++++++-- + credential.h | 16 ++++++++++++++++ + t/t0300-credentials.sh | 12 ++++++++++-- + 3 files changed, 60 insertions(+), 4 deletions(-) + +diff --git a/credential.c b/credential.c +index 00ee4d6..eeeac32 100644 +--- a/credential.c ++++ b/credential.c +@@ -321,7 +321,22 @@ void credential_reject(struct credential *c) + c->approved = 0; + } + +-void credential_from_url(struct credential *c, const char *url) ++static int check_url_component(const char *url, int quiet, ++ const char *name, const char *value) ++{ ++ if (!value) ++ return 0; ++ if (!strchr(value, '\n')) ++ return 0; ++ ++ if (!quiet) ++ warning(_("url contains a newline in its %s component: %s"), ++ name, url); ++ return -1; ++} ++ ++int credential_from_url_gently(struct credential *c, const char *url, ++ int quiet) + { + const char *at, *colon, *cp, *slash, *host, *proto_end; + +@@ -335,7 +350,7 @@ void credential_from_url(struct credential *c, const char *url) + */ + proto_end = strstr(url, "://"); + if (!proto_end) +- return; ++ return 0; + cp = proto_end + 3; + at = strchr(cp, '@'); + colon = strchr(cp, ':'); +@@ -370,4 +385,21 @@ void credential_from_url(struct credential *c, const char *url) + while (p > c->path && *p == '/') + *p-- = '\0'; + } ++ ++ if (check_url_component(url, quiet, "username", c->username) < 0 || ++ check_url_component(url, quiet, "password", c->password) < 0 || ++ check_url_component(url, quiet, "protocol", c->protocol) < 0 || ++ check_url_component(url, quiet, "host", c->host) < 0 || ++ check_url_component(url, quiet, "path", c->path) < 0) ++ return -1; ++ ++ return 0; ++} ++ ++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); ++ } + } +diff --git a/credential.h b/credential.h +index 6b0cd16..122a23c 100644 +--- a/credential.h ++++ b/credential.h +@@ -28,7 +28,23 @@ struct credential { + + int credential_read(struct credential *, FILE *); + void credential_write(const struct credential *, FILE *); ++ ++/* ++ * Parse a url into a credential struct, replacing any existing contents. ++ * ++ * Ifthe url can't be parsed (e.g., a missing "proto://" component), the ++ * resulting credential will be empty but we'll still return success from the ++ * "gently" form. ++ * ++ * If we encounter a component which cannot be represented as a credential ++ * value (e.g., because it contains a newline), the "gently" form will return ++ * an error but leave the broken state in the credential object for further ++ * examination. The non-gentle form will issue a warning to stderr and return ++ * an empty credential. ++ */ + void credential_from_url(struct credential *, const char *url); ++int credential_from_url_gently(struct credential *, const char *url, int quiet); ++ + int credential_match(const struct credential *have, + const struct credential *want); + +diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh +index 15cc3c5..3bec445 100755 +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -309,9 +309,17 @@ test_expect_success 'empty helper spec resets helper list' ' + EOF + ' + +-test_expect_success 'url parser rejects embedded newlines' ' +- test_must_fail git credential fill <<-\EOF ++test_expect_success 'url parser ignores embedded newlines' ' ++ check fill <<-EOF + url=https://one.example.com?%0ahost=two.example.com/ ++ -- ++ username=askpass-username ++ password=askpass-password ++ -- ++ 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: + EOF + ' + +-- +1.8.3.1 + diff --git a/fsck-detect-gitmodules-URLs-with-embedded-newlines.patch b/fsck-detect-gitmodules-URLs-with-embedded-newlines.patch new file mode 100644 index 0000000000000000000000000000000000000000..6adad1f34751aa5e28aac46117683a3369ee9c13 --- /dev/null +++ b/fsck-detect-gitmodules-URLs-with-embedded-newlines.patch @@ -0,0 +1,99 @@ +From 07259e74ec1237c836874342c65650bdee8a3993 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Wed, 11 Mar 2020 18:48:24 -0400 +Subject: [PATCH] fsck: detect gitmodules URLs with embedded newlines + +The credential protocol can't handle values with newlines. We already +detect and block any such URLs from being used with credential helpers, +but let's also add an fsck check to detect and block gitmodules files +with such URLs. That will let us notice the problem earlier when +transfer.fsckObjects is turned on. And in particular it will prevent bad +objects from spreading, which may protect downstream users running older +versions of Git. + +We'll file this under the existing gitmodulesUrl flag, which covers URLs +with option injection. There's really no need to distinguish the exact +flaw in the URL in this context. Likewise, I've expanded the description +of t7416 to cover all types of bogus URLs. +--- + fsck.c | 16 +++++++++++++++- + t/t7416-submodule-dash-url.sh | 18 +++++++++++++++++- + 2 files changed, 32 insertions(+), 2 deletions(-) + +diff --git a/fsck.c b/fsck.c +index 0741e62..5b437c2 100644 +--- a/fsck.c ++++ b/fsck.c +@@ -14,6 +14,7 @@ + #include "packfile.h" + #include "submodule-config.h" + #include "config.h" ++#include "credential.h" + #include "help.h" + + static struct oidset gitmodules_found = OIDSET_INIT; +@@ -941,6 +942,19 @@ static int fsck_tag(struct tag *tag, const char *data, + return fsck_tag_buffer(tag, data, size, options); + } + ++static int check_submodule_url(const char *url) ++{ ++ struct credential c = CREDENTIAL_INIT; ++ int ret; ++ ++ if (looks_like_command_line_option(url)) ++ return -1; ++ ++ ret = credential_from_url_gently(&c, url, 1); ++ credential_clear(&c); ++ return ret; ++} ++ + struct fsck_gitmodules_data { + struct object *obj; + struct fsck_options *options; +@@ -965,7 +979,7 @@ static int fsck_gitmodules_fn(const char *var, const char *value, void *vdata) + "disallowed submodule name: %s", + name); + if (!strcmp(key, "url") && value && +- looks_like_command_line_option(value)) ++ check_submodule_url(value) < 0) + data->ret |= report(data->options, data->obj, + FSCK_MSG_GITMODULES_URL, + "disallowed submodule url: %s", +diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh +index 5ba041f..41431b1 100755 +--- a/t/t7416-submodule-dash-url.sh ++++ b/t/t7416-submodule-dash-url.sh +@@ -1,6 +1,6 @@ + #!/bin/sh + +-test_description='check handling of .gitmodule url with dash' ++test_description='check handling of disallowed .gitmodule urls' + . ./test-lib.sh + + test_expect_success 'create submodule with protected dash in url' ' +@@ -60,4 +60,20 @@ test_expect_success 'trailing backslash is handled correctly' ' + test_i18ngrep ! "unknown option" err + ' + ++test_expect_success 'fsck rejects embedded newline in url' ' ++ # create an orphan branch to avoid existing .gitmodules objects ++ git checkout --orphan newline && ++ cat >.gitmodules <<-\EOF && ++ [submodule "foo"] ++ url = "https://one.example.com?%0ahost=two.example.com/foo.git" ++ EOF ++ git add .gitmodules && ++ git commit -m "gitmodules 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 +-- +1.8.3.1 + diff --git a/git.spec b/git.spec index 5efd84512274f17c7910bf92cdf4e6daa123b3d3..f7a11dd5ffe59931e344fe490ff50cb19f0747fe 100644 --- a/git.spec +++ b/git.spec @@ -1,7 +1,7 @@ %global gitexecdir %{_libexecdir}/git-core Name: git Version: 2.23.0 -Release: 15 +Release: 16 Summary: A popular and widely used Version Control System License: GPLv2+ or LGPLv2.1 URL: https://git-scm.com/ @@ -30,6 +30,17 @@ Patch13: CVE-2019-19604.patch # suite, so we add this patch to skip the updating of preference Patch14: skip-updating-the-preference.patch Patch15: CVE-2020-5260.patch +Patch16: credential-detect-unrepresentable-values-when-parsin.patch +Patch17: fsck-detect-gitmodules-URLs-with-embedded-newlines.patch +Patch18: CVE-2020-11008-1.patch +Patch19: CVE-2020-11008-2.patch +Patch20: CVE-2020-11008-3.patch +Patch21: CVE-2020-11008-4.patch +Patch22: CVE-2020-11008-5.patch +Patch23: CVE-2020-11008-6.patch +Patch24: CVE-2020-11008-7.patch +Patch25: CVE-2020-11008-8.patch +Patch26: CVE-2020-11008-9.patch BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre-devel desktop-file-utils BuildRequires: python3-devel perl-generators perl-interpreter perl-Error perl(Test::More) perl-MailTools perl(Test) gdb @@ -277,6 +288,14 @@ make test %{_mandir}/man7/git*.7.* %changelog +* Thu May 14 2020 gaihuiying - 2.23.0-16 +- Type:cves +- ID:CVE-2020-11008 +- SUG:NA +- DESC:fix CVE-2020-11008 + credential: detect unrepresentable values when parsin urls + fsck: detect gitmodules URLs with embedded newlines + * Fri Apr 17 2020 openEuler Buildteam - 2.23.0-15 - Type:cves - ID:CVE-2020-5260