diff options
| -rw-r--r-- | Documentation/RelNotes/2.17.4.txt | 16 | ||||
| -rw-r--r-- | Documentation/RelNotes/2.18.3.txt | 5 | ||||
| -rwxr-xr-x | GIT-VERSION-GEN | 2 | ||||
| l--------- | RelNotes | 2 | ||||
| -rw-r--r-- | credential.c | 38 | ||||
| -rw-r--r-- | credential.h | 16 | ||||
| -rw-r--r-- | fsck.c | 16 | ||||
| -rwxr-xr-x | t/lib-credential.sh | 2 | ||||
| -rwxr-xr-x | t/t0300-credentials.sh | 14 | ||||
| -rwxr-xr-x | t/t7416-submodule-dash-url.sh | 18 |
10 files changed, 122 insertions, 7 deletions
diff --git a/Documentation/RelNotes/2.17.4.txt b/Documentation/RelNotes/2.17.4.txt new file mode 100644 index 0000000000..7d794ca01a --- /dev/null +++ b/Documentation/RelNotes/2.17.4.txt @@ -0,0 +1,16 @@ +Git v2.17.4 Release Notes +========================= + +This release is to address the security issue: CVE-2020-5260 + +Fixes since v2.17.3 +------------------- + + * With a crafted URL that contains a newline in it, the credential + helper machinery can be fooled to give credential information for + a wrong host. The attack has been made impossible by forbidding + a newline character in any value passed via the credential + protocol. + +Credit for finding the vulnerability goes to Felix Wilhelm of Google +Project Zero. diff --git a/Documentation/RelNotes/2.18.3.txt b/Documentation/RelNotes/2.18.3.txt new file mode 100644 index 0000000000..25143f0cec --- /dev/null +++ b/Documentation/RelNotes/2.18.3.txt @@ -0,0 +1,5 @@ +Git v2.18.3 Release Notes +========================= + +This release merges the security fix that appears in v2.17.4; see +the release notes for that version for details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 5ae6419eb4..fd9e5b87b2 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.18.2 +DEF_VER=v2.18.3 LF=' ' @@ -1 +1 @@ -Documentation/RelNotes/2.18.2.txt
\ No newline at end of file +Documentation/RelNotes/2.18.3.txt
\ No newline at end of file diff --git a/credential.c b/credential.c index 62be651b03..24823829f6 100644 --- a/credential.c +++ b/credential.c @@ -195,6 +195,8 @@ static void credential_write_item(FILE *fp, const char *key, const char *value) { if (!value) return; + if (strchr(value, '\n')) + die("credential value for %s contains newline", key); fprintf(fp, "%s=%s\n", key, value); } @@ -322,7 +324,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; @@ -336,7 +353,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, ':'); @@ -371,4 +388,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 6b0cd16be2..122a23cd2f 100644 --- 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); @@ -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; @@ -945,6 +946,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; @@ -969,7 +983,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/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() { diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh index 03bd31e9f2..3bec445cac 100755 --- a/t/t0300-credentials.sh +++ b/t/t0300-credentials.sh @@ -309,4 +309,18 @@ test_expect_success 'empty helper spec resets helper list' ' 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 +' + test_done diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh index 5ba041f537..41431b1ac3 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 |