Commit 802f66ea authored by Natanael Copa's avatar Natanael Copa
Browse files

main/git: security fix (CVE-2018-17456)

Patch from Debian.

fixes #9544
parent 7838ef4a
......@@ -2,7 +2,7 @@
# Maintainer: Natanael Copa <ncopa@alpinelinux.org>
pkgname=git
pkgver=2.11.3
pkgrel=1
pkgrel=2
pkgdesc="A distributed version control system"
url="https://www.git-scm.com/"
arch="all"
......@@ -36,6 +36,7 @@ source="https://www.kernel.org/pub/software/scm/git/git-$pkgver.tar.xz
git-daemon.confd
CVE-2018-11233.patch
CVE-2018-11235.patch
CVE-2018-17456.patch
"
_makeopts="
......@@ -49,6 +50,8 @@ _gitcoredir=/usr/libexec/git-core
_builddir="$srcdir"/$pkgname-$pkgver
# secfixes:
# 2.11.3-r2:
# - CVE-2018-17456
# 2.11.3-r1:
# - CVE-2018-11233
# - CVE-2018-11235
......@@ -229,16 +232,19 @@ md5sums="1e961079c5a38eb6a79f4a479550aade git-2.11.3.tar.xz
8bb51e7742a171891a51967b84c8088e git-daemon.initd
33427921f86acc26d1578d8b308e5baa git-daemon.confd
c46121013e7329a583c1d0d885b4a5bd CVE-2018-11233.patch
af7b89141594387b0d05c13cfbb43e5b CVE-2018-11235.patch"
af7b89141594387b0d05c13cfbb43e5b CVE-2018-11235.patch
b9a35c4795f69c280ab56e47dcfc8282 CVE-2018-17456.patch"
sha256sums="7343bbd489f59531d66bc086393f0d5f530b5175927c29fb97b07f9d2cbc31ac git-2.11.3.tar.xz
968e996a306dab643970c5ce1ac40926146b01b9c38a8fe81c74340a0302dbc7 bb-tar.patch
ff9b5beefbe55ba6340f1c4acced195515002d0ccb431a8fcd5b1078eacd2e59 git-daemon.initd
4703ba2372c661fb674a29fea7f64983f8b1b3136d971663509249655bca6e21 git-daemon.confd
93d9e1f4df65da8237fa1c93e544ad3a807a9dcabaf167644893b02465ff0080 CVE-2018-11233.patch
7c0171e4258901bf154be43ba357888a27b1f7e618477704c413e60b1c0e15d5 CVE-2018-11235.patch"
7c0171e4258901bf154be43ba357888a27b1f7e618477704c413e60b1c0e15d5 CVE-2018-11235.patch
e551997905ef35ef91e8c77101e900056d15dfd466e108acebd42dd9a3e85ff9 CVE-2018-17456.patch"
sha512sums="13f7dc80031c4376839debc0d900d1a6e79c1fe9d7cd4abe045ddac73bab059222abeb4ab6b9ccb06f11d6dc3ea4a219b653b679fb8142b75f4a4f820ecb4df4 git-2.11.3.tar.xz
85767b5e03137008d6a96199e769e3979f75d83603ac8cb13a3481a915005637409a4fd94e0720da2ec6cd1124f35eba7cf20109a94816c4b4898a81fbc46bd2 bb-tar.patch
89528cdd14c51fd568aa61cf6c5eae08ea0844e59f9af9292da5fc6c268261f4166017d002d494400945e248df6b844e2f9f9cd2d9345d516983f5a110e4c42a git-daemon.initd
fbf1f425206a76e2a8f82342537ed939ff7e623d644c086ca2ced5f69b36734695f9f80ebda1728f75a94d6cd2fcb71bf845b64239368caab418e4d368c141ec git-daemon.confd
5201ff46b774a4f88fe8c4162394311ed85f4f507ebae1bce71e4dc199bb5ffc60f38c718bb6343c660560b93e648778fc6c4ad3917f95f0f44e31aecc6a6e38 CVE-2018-11233.patch
b09b988ecf6cd2814b44120509cb455f1c2694641d50b62762861708d27e8bf5dc7af5fcb2e5acf589e005b98b9ba04d7bdef5c1e02bd66470eb3faab16cbf3d CVE-2018-11235.patch"
b09b988ecf6cd2814b44120509cb455f1c2694641d50b62762861708d27e8bf5dc7af5fcb2e5acf589e005b98b9ba04d7bdef5c1e02bd66470eb3faab16cbf3d CVE-2018-11235.patch
bb17113ac695c0976dea7046e3267ea8328dbb3b14a543934f15e50de9e238a0c88cc1f91b15f37006534cf67076b27e9be1642f242dabac743d896a446d3bba CVE-2018-17456.patch"
From 13822f2a3a5ae7c326333c4d6186feb24128a493 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 24 Apr 2017 20:57:47 -0400
Subject: submodule_init: die cleanly on submodules without url defined
commit 627fde102515a7807dba89acaa88cb053b38a44a upstream.
When we init a submodule, we try to die when it has no URL
defined:
url = xstrdup(sub->url);
if (!url)
die(...);
But that's clearly nonsense. xstrdup() will never return
NULL, and if sub->url is NULL, we'll segfault.
These two bits of code need to be flipped, so we check
sub->url before looking at it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/submodule--helper.c | 6 +++---
t/t7400-submodule-basic.sh | 8 ++++++++
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b4278e3a71..25555393f1 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -337,12 +337,12 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
strbuf_reset(&sb);
strbuf_addf(&sb, "submodule.%s.url", sub->name);
if (git_config_get_string(sb.buf, &url)) {
- url = xstrdup(sub->url);
-
- if (!url)
+ if (!sub->url)
die(_("No url found for submodule path '%s' in .gitmodules"),
displaypath);
+ url = xstrdup(sub->url);
+
/* Possibly a url relative to parent */
if (starts_with_dot_dot_slash(url) ||
starts_with_dot_slash(url)) {
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index b77cce8e40..db82d8d81c 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -38,6 +38,14 @@ test_expect_success 'submodule update aborts on missing .gitmodules file' '
test_i18ngrep "Submodule path .sub. not initialized" actual
'
+test_expect_success 'submodule update aborts on missing gitmodules url' '
+ test_when_finished "git update-index --remove sub" &&
+ git update-index --add --cacheinfo 160000,$(git rev-parse HEAD),sub &&
+ test_when_finished "rm -f .gitmodules" &&
+ git config -f .gitmodules submodule.s.path sub &&
+ test_must_fail git submodule init
+'
+
test_expect_success 'configuration parsing' '
test_when_finished "rm -f .gitmodules" &&
cat >.gitmodules <<-\EOF &&
--
2.19.0.605.g01d371f741
From 56b40fee797d4eb887a71e2fdb443773398a33bf Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 24 Sep 2018 04:32:15 -0400
Subject: submodule--helper: use "--" to signal end of clone options
commit 98afac7a7cefdca0d2c4917dd8066a59f7088265 upstream.
When we clone a submodule, we call "git clone $url $path".
But there's nothing to say that those components can't begin
with a dash themselves, confusing git-clone into thinking
they're options. Let's pass "--" to make it clear what we
expect.
There's no test here, because it's actually quite hard to
make these names work, even with "git clone" parsing them
correctly. And we're going to restrict these cases even
further in future commits. So we'll leave off testing until
then; this is just the minimal fix to prevent us from doing
something stupid with a badly formed entry.
Reported-by: joernchen <joernchen@phenoelit.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/submodule--helper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 25555393f1..b6801b6c7a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -469,6 +469,7 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
if (gitdir && *gitdir)
argv_array_pushl(&cp.args, "--separate-git-dir", gitdir, NULL);
+ argv_array_push(&cp.args, "--");
argv_array_push(&cp.args, url);
argv_array_push(&cp.args, path);
--
2.19.0.605.g01d371f741
From 89a75e19b8f091ea687750aa09fb83eb6c9a3f67 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 24 Sep 2018 04:36:30 -0400
Subject: submodule-config: ban submodule urls that start with dash
commit f6adec4e329ef0e25e14c63b735a5956dc67b8bc upstream.
The previous commit taught the submodule code to invoke our
"git clone $url $path" with a "--" separator so that we
aren't confused by urls or paths that start with dashes.
However, that's just one code path. It's not clear if there
are others, and it would be an easy mistake to add one in
the future. Moreover, even with the fix in the previous
commit, it's quite hard to actually do anything useful with
such an entry. Any url starting with a dash must fall into
one of three categories:
- it's meant as a file url, like "-path". But then any
clone is not going to have the matching path, since it's
by definition relative inside the newly created clone. If
you spell it as "./-path", the submodule code sees the
"/" and translates this to an absolute path, so it at
least works (assuming the receiver has the same
filesystem layout as you). But that trick does not apply
for a bare "-path".
- it's meant as an ssh url, like "-host:path". But this
already doesn't work, as we explicitly disallow ssh
hostnames that begin with a dash (to avoid option
injection against ssh).
- it's a remote-helper scheme, like "-scheme::data". This
_could_ work if the receiver bends over backwards and
creates a funny-named helper like "git-remote--scheme".
But normally there would not be any helper that matches.
Since such a url does not work today and is not likely to do
anything useful in the future, let's simply disallow them
entirely. That protects the existing "git clone" path (in a
belt-and-suspenders way), along with any others that might
exist.
Our tests cover two cases:
1. A file url with "./" continues to work, showing that
there's an escape hatch for people with truly silly
repo names.
2. A url starting with "-" is rejected.
Note that we expect case (2) to fail, but it would have done
so even without this commit, for the reasons given above.
So instead of just expecting failure, let's also check for
the magic word "ignoring" on stderr. That lets us know that
we failed for the right reason.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
submodule-config.c | 8 ++++++++
t/t7416-submodule-dash-url.sh | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)
create mode 100755 t/t7416-submodule-dash-url.sh
diff --git a/submodule-config.c b/submodule-config.c
index 2697f7a145..3ea69303f9 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -305,6 +305,12 @@ static void warn_multiple_config(const unsigned char *commit_sha1,
commit_string, name, option);
}
+static void warn_command_line_option(const char *var, const char *value)
+{
+ warning(_("ignoring '%s' which may be interpreted as"
+ " a command-line option: %s"), var, value);
+}
+
struct parse_config_parameter {
struct submodule_cache *cache;
const unsigned char *commit_sha1;
@@ -370,6 +376,8 @@ static int parse_config(const char *var, const char *value, void *data)
} else if (!strcmp(item.buf, "url")) {
if (!value) {
ret = config_error_nonbool(var);
+ } else if (looks_like_command_line_option(value)) {
+ warn_command_line_option(var, value);
} else if (!me->overwrite && submodule->url) {
warn_multiple_config(me->commit_sha1, submodule->name,
"url");
diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh
new file mode 100755
index 0000000000..459193c976
--- /dev/null
+++ b/t/t7416-submodule-dash-url.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='check handling of .gitmodule url with dash'
+. ./test-lib.sh
+
+test_expect_success 'create submodule with protected dash in url' '
+ git init upstream &&
+ git -C upstream commit --allow-empty -m base &&
+ mv upstream ./-upstream &&
+ git submodule add ./-upstream sub &&
+ git add sub .gitmodules &&
+ git commit -m submodule
+'
+
+test_expect_success 'clone can recurse submodule' '
+ test_when_finished "rm -rf dst" &&
+ git clone --recurse-submodules . dst &&
+ echo base >expect &&
+ git -C dst/sub log -1 --format=%s >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'remove ./ protection from .gitmodules url' '
+ perl -i -pe "s{\./}{}" .gitmodules &&
+ git commit -am "drop protection"
+'
+
+test_expect_success 'clone rejects unprotected dash' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --recurse-submodules . dst 2>err &&
+ test_i18ngrep ignoring err
+'
+
+test_done
--
2.19.0.605.g01d371f741
From 8554768479aae958c5f738ddc95419a19392682a Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 24 Sep 2018 04:39:55 -0400
Subject: submodule-config: ban submodule paths that start with a dash
commit 273c61496f88c6495b886acb1041fe57965151da upstream.
We recently banned submodule urls that look like
command-line options. This is the matching change to ban
leading-dash paths.
As with the urls, this should not break any use cases that
currently work. Even with our "--" separator passed to
git-clone, git-submodule.sh gets confused. Without the code
portion of this patch, the clone of "-sub" added in t7417
would yield results like:
/path/to/git-submodule: 410: cd: Illegal option -s
/path/to/git-submodule: 417: cd: Illegal option -s
/path/to/git-submodule: 410: cd: Illegal option -s
/path/to/git-submodule: 417: cd: Illegal option -s
Fetched in submodule path '-sub', but it did not contain b56243f8f4eb91b2f1f8109452e659f14dd3fbe4. Direct fetching of that commit failed.
Moreover, naively adding such a submodule doesn't work:
$ git submodule add $url -sub
The following path is ignored by one of your .gitignore files:
-sub
even though there is no such ignore pattern (the test script
hacks around this with a well-placed "git mv").
Unlike leading-dash urls, though, it's possible that such a
path _could_ be useful if we eventually made it work. So
this commit should be seen not as recommending a particular
policy, but rather temporarily closing off a broken and
possibly dangerous code-path. We may revisit this decision
later.
There are two minor differences to the tests in t7416 (that
covered urls):
1. We don't have a "./-sub" escape hatch to make this
work, since the submodule code expects to be able to
match canonical index names to the path field (so you
are free to add submodule config with that path, but we
would never actually use it, since an index entry would
never start with "./").
2. After this patch, cloning actually succeeds. Since we
ignore the submodule.*.path value, we fail to find a
config stanza for our submodule at all, and simply
treat it as inactive. We still check for the "ignoring"
message.
[jn: the original patch expects 'git clone' to succeed in
the test because v2.13.0-rc0~10^2~3 (clone: teach
--recurse-submodules to optionally take a pathspec,
2017-03-17) makes 'git clone' skip invalid submodules.
Updated the test to pass in older Git versions where the
submodule name check makes 'git clone' fail.]
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
submodule-config.c | 2 ++
t/t7417-submodule-path-url.sh | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+)
create mode 100755 t/t7417-submodule-path-url.sh
diff --git a/submodule-config.c b/submodule-config.c
index 3ea69303f9..3be591783e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -336,6 +336,8 @@ static int parse_config(const char *var, const char *value, void *data)
if (!strcmp(item.buf, "path")) {
if (!value)
ret = config_error_nonbool(var);
+ else if (looks_like_command_line_option(value))
+ warn_command_line_option(var, value);
else if (!me->overwrite && submodule->path)
warn_multiple_config(me->commit_sha1, submodule->name,
"path");
diff --git a/t/t7417-submodule-path-url.sh b/t/t7417-submodule-path-url.sh
new file mode 100755
index 0000000000..894ed51685
--- /dev/null
+++ b/t/t7417-submodule-path-url.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+test_description='check handling of .gitmodule path with dash'
+. ./test-lib.sh
+
+test_expect_success 'create submodule with dash in path' '
+ git init upstream &&
+ git -C upstream commit --allow-empty -m base &&
+ git submodule add ./upstream sub &&
+ git mv sub ./-sub &&
+ git commit -m submodule
+'
+
+test_expect_success 'clone rejects unprotected dash' '
+ test_when_finished "rm -rf dst" &&
+ test_might_fail git clone --recurse-submodules . dst 2>err &&
+ test_i18ngrep ignoring err
+'
+
+test_done
--
2.19.0.605.g01d371f741
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment