diff -Nru git-2.17.1/debian/changelog git-2.17.1/debian/changelog --- git-2.17.1/debian/changelog 2019-12-09 13:29:20.000000000 +0000 +++ git-2.17.1/debian/changelog 2020-04-10 15:59:06.000000000 +0000 @@ -1,3 +1,19 @@ +git (1:2.17.1-1ubuntu0.6) bionic-security; urgency=medium + + * SECURITY UPDATE: credential helper issue with newlines in URL + - debian/patches/CVE-2020-5260-1.patch: avoid writing values with + newlines in credential.c, t/t0300-credentials.sh. + - debian/patches/CVE-2020-5260-2.patch: use test_i18ncmp to check + stderr in t/lib-credential.sh. + - debian/patches/CVE-2020-5260-3.patch: detect unrepresentable values + when parsing urls in credential.c, credential.h, + t/t0300-credentials.sh. + - debian/patches/CVE-2020-5260-4.patch: detect gitmodules URLs with + embedded newlines in fsck.c, t/t7416-submodule-dash-url.sh. + - CVE-2020-5260 + + -- Marc Deslauriers Fri, 10 Apr 2020 11:59:06 -0400 + git (1:2.17.1-1ubuntu0.5) bionic-security; urgency=medium * SECURITY UPDATE: Multiple security issues diff -Nru git-2.17.1/debian/patches/CVE-2020-5260-1.patch git-2.17.1/debian/patches/CVE-2020-5260-1.patch --- git-2.17.1/debian/patches/CVE-2020-5260-1.patch 1970-01-01 00:00:00.000000000 +0000 +++ git-2.17.1/debian/patches/CVE-2020-5260-1.patch 2020-04-10 15:58:16.000000000 +0000 @@ -0,0 +1,54 @@ +From 9a6bbee8006c24b46a85d29e7b38cfa79e9ab21b Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Wed, 11 Mar 2020 17:53:41 -0400 +Subject: [PATCH 1/4] credential: avoid writing values with newlines + +The credential protocol that we use to speak to helpers can't represent +values with newlines in them. This was an intentional design choice to +keep the protocol simple, since none of the values we pass should +generally have newlines. + +However, if we _do_ encounter a newline in a value, we blindly transmit +it in credential_write(). Such values may break the protocol syntax, or +worse, inject new valid lines into the protocol stream. + +The most likely way for a newline to end up in a credential struct is by +decoding a URL with a percent-encoded newline. However, since the bug +occurs at the moment we write the value to the protocol, we'll catch it +there. That should leave no possibility of accidentally missing a code +path that can trigger the problem. + +At this level of the code we have little choice but to die(). However, +since we'd not ever expect to see this case outside of a malicious URL, +that's an acceptable outcome. + +Reported-by: Felix Wilhelm +--- + credential.c | 2 ++ + t/t0300-credentials.sh | 6 ++++++ + 2 files changed, 8 insertions(+) + +--- a/credential.c ++++ b/credential.c +@@ -194,6 +194,8 @@ static void credential_write_item(FILE * + { + if (!value) + return; ++ if (strchr(value, '\n')) ++ die("credential value for %s contains newline", key); + fprintf(fp, "%s=%s\n", key, value); + } + +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -309,4 +309,10 @@ test_expect_success 'empty helper spec r + EOF + ' + ++test_expect_success 'url parser rejects embedded newlines' ' ++ test_must_fail git credential fill <<-\EOF ++ url=https://one.example.com?%0ahost=two.example.com/ ++ EOF ++' ++ + test_done diff -Nru git-2.17.1/debian/patches/CVE-2020-5260-2.patch git-2.17.1/debian/patches/CVE-2020-5260-2.patch --- git-2.17.1/debian/patches/CVE-2020-5260-2.patch 1970-01-01 00:00:00.000000000 +0000 +++ git-2.17.1/debian/patches/CVE-2020-5260-2.patch 2020-04-10 15:58:18.000000000 +0000 @@ -0,0 +1,31 @@ +From 17f1c0b8c7e447aa62f85dc355bb48133d2812f2 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Wed, 11 Mar 2020 18:11:37 -0400 +Subject: [PATCH 2/4] t/lib-credential: use test_i18ncmp to check stderr + +The credential tests have a "check" function which feeds some input to +git-credential and checks the stdout and stderr. We look for exact +matches in the output. For stdout, this makes sense; the output is +the credential protocol. But for stderr, we may be showing various +diagnostic messages, or the prompts fed to the askpass program, which +could be translated. Let's mark them as such. +--- + t/lib-credential.sh | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/t/lib-credential.sh b/t/lib-credential.sh +index 937b831ea6..bb88cc0108 100755 +--- a/t/lib-credential.sh ++++ b/t/lib-credential.sh +@@ -19,7 +19,7 @@ check() { + false + fi && + test_cmp expect-stdout stdout && +- test_cmp expect-stderr stderr ++ test_i18ncmp expect-stderr stderr + } + + read_chunk() { +-- +2.17.1 + diff -Nru git-2.17.1/debian/patches/CVE-2020-5260-3.patch git-2.17.1/debian/patches/CVE-2020-5260-3.patch --- git-2.17.1/debian/patches/CVE-2020-5260-3.patch 1970-01-01 00:00:00.000000000 +0000 +++ git-2.17.1/debian/patches/CVE-2020-5260-3.patch 2020-04-10 15:58:29.000000000 +0000 @@ -0,0 +1,143 @@ +From c716fe4bd917e013bf376a678b3a924447777b2d Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Thu, 12 Mar 2020 01:31:11 -0400 +Subject: [PATCH 3/4] 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(-) + +--- a/credential.c ++++ b/credential.c +@@ -321,7 +321,22 @@ void credential_reject(struct credential + 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 credenti + */ + 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 credenti + 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); ++ } + } +--- a/credential.h ++++ b/credential.h +@@ -28,7 +28,23 @@ void credential_reject(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); + +--- a/t/t0300-credentials.sh ++++ b/t/t0300-credentials.sh +@@ -309,9 +309,17 @@ test_expect_success 'empty helper spec r + 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 + ' + diff -Nru git-2.17.1/debian/patches/CVE-2020-5260-4.patch git-2.17.1/debian/patches/CVE-2020-5260-4.patch --- git-2.17.1/debian/patches/CVE-2020-5260-4.patch 1970-01-01 00:00:00.000000000 +0000 +++ git-2.17.1/debian/patches/CVE-2020-5260-4.patch 2020-04-10 15:58:53.000000000 +0000 @@ -0,0 +1,94 @@ +Backport of: + +From 07259e74ec1237c836874342c65650bdee8a3993 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Wed, 11 Mar 2020 18:48:24 -0400 +Subject: [PATCH 4/4] 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(-) + +--- a/fsck.c ++++ b/fsck.c +@@ -14,6 +14,7 @@ + #include "packfile.h" + #include "submodule-config.h" + #include "config.h" ++#include "credential.h" + + static struct oidset gitmodules_found = OIDSET_INIT; + static struct oidset gitmodules_done = OIDSET_INIT; +@@ -941,6 +942,19 @@ static int fsck_tag(struct tag *tag, con + 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 + "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", +--- 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 + 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 diff -Nru git-2.17.1/debian/patches/series git-2.17.1/debian/patches/series --- git-2.17.1/debian/patches/series 2019-12-09 13:28:49.000000000 +0000 +++ git-2.17.1/debian/patches/series 2020-04-10 15:58:33.000000000 +0000 @@ -33,3 +33,7 @@ CVE-2019-13xx/0027-mingw-refuse-to-access-paths-with-trailing-spaces-or.patch CVE-2019-13xx/0028-mingw-refuse-to-access-paths-with-illegal-characters.patch CVE-2019-13xx/0029-mingw-handle-subst-ed-DOS-drives.patch +CVE-2020-5260-1.patch +CVE-2020-5260-2.patch +CVE-2020-5260-3.patch +CVE-2020-5260-4.patch