submodule-config: ban submodule urls that start with dash
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>
2018-09-24 08:36:30 +00:00
|
|
|
#!/bin/sh
|
|
|
|
|
|
2020-03-11 22:48:24 +00:00
|
|
|
test_description='check handling of disallowed .gitmodule urls'
|
submodule-config: ban submodule urls that start with dash
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>
2018-09-24 08:36:30 +00:00
|
|
|
. ./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
|
|
|
|
|
'
|
|
|
|
|
|
2018-09-24 08:37:17 +00:00
|
|
|
test_expect_success 'fsck accepts protected dash' '
|
|
|
|
|
test_when_finished "rm -rf dst" &&
|
|
|
|
|
git init --bare dst &&
|
|
|
|
|
git -C dst config transfer.fsckObjects true &&
|
|
|
|
|
git push dst HEAD
|
|
|
|
|
'
|
|
|
|
|
|
submodule-config: ban submodule urls that start with dash
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>
2018-09-24 08:36:30 +00:00
|
|
|
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
|
|
|
|
|
'
|
|
|
|
|
|
2018-09-24 08:37:17 +00:00
|
|
|
test_expect_success 'fsck rejects unprotected dash' '
|
|
|
|
|
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
|
|
|
|
|
'
|
|
|
|
|
|
mingw: fix quoting of arguments
We need to be careful to follow proper quoting rules. For example, if an
argument contains spaces, we have to quote them. Double-quotes need to
be escaped. Backslashes need to be escaped, but only if they are
followed by a double-quote character.
We need to be _extra_ careful to consider the case where an argument
ends in a backslash _and_ needs to be quoted: in this case, we append a
double-quote character, i.e. the backslash now has to be escaped!
The current code, however, fails to recognize that, and therefore can
turn an argument that ends in a single backslash into a quoted argument
that now ends in an escaped double-quote character. This allows
subsequent command-line parameters to be split and part of them being
mistaken for command-line options, e.g. through a maliciously-crafted
submodule URL during a recursive clone.
Technically, we would not need to quote _all_ arguments which end in a
backslash _unless_ the argument needs to be quoted anyway. For example,
`test\` would not need to be quoted, while `test \` would need to be.
To keep the code simple, however, and therefore easier to reason about
and ensure its correctness, we now _always_ quote an argument that ends
in a backslash.
This addresses CVE-2019-1350.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-09-13 14:32:43 +00:00
|
|
|
test_expect_success 'trailing backslash is handled correctly' '
|
|
|
|
|
git init testmodule &&
|
|
|
|
|
test_commit -C testmodule c &&
|
|
|
|
|
git submodule add ./testmodule &&
|
|
|
|
|
: ensure that the name ends in a double backslash &&
|
|
|
|
|
sed -e "s|\\(submodule \"testmodule\\)\"|\\1\\\\\\\\\"|" \
|
|
|
|
|
-e "s|url = .*|url = \" --should-not-be-an-option\"|" \
|
|
|
|
|
<.gitmodules >.new &&
|
|
|
|
|
mv .new .gitmodules &&
|
|
|
|
|
git commit -am "Add testmodule" &&
|
|
|
|
|
test_must_fail git clone --verbose --recurse-submodules . dolly 2>err &&
|
|
|
|
|
test_i18ngrep ! "unknown option" err
|
|
|
|
|
'
|
|
|
|
|
|
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 <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
2020-04-19 03:54:13 +00:00
|
|
|
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
|
|
|
|
|
'
|
|
|
|
|
|
2020-04-19 03:54:57 +00:00
|
|
|
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
|
|
|
|
|
'
|
|
|
|
|
|
2020-04-19 03:57:22 +00:00
|
|
|
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
|
|
|
|
|
'
|
|
|
|
|
|
2020-04-19 03:52:34 +00:00
|
|
|
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
|
|
|
|
|
'
|
|
|
|
|
|
2020-03-11 22:48:24 +00:00
|
|
|
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
|
|
|
|
|
'
|
|
|
|
|
|
2020-04-19 03:52:34 +00:00
|
|
|
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
|
|
|
|
|
'
|
|
|
|
|
|
2021-01-07 09:44:17 +00:00
|
|
|
test_expect_success 'fsck rejects embedded newline in git url' '
|
|
|
|
|
git checkout --orphan git-newline &&
|
|
|
|
|
cat >.gitmodules <<-\EOF &&
|
|
|
|
|
[submodule "foo"]
|
|
|
|
|
url = "git://example.com:1234/repo%0a.git"
|
|
|
|
|
EOF
|
|
|
|
|
git add .gitmodules &&
|
|
|
|
|
git commit -m "git 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
|
|
|
|
|
'
|
|
|
|
|
|
submodule-config: ban submodule urls that start with dash
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>
2018-09-24 08:36:30 +00:00
|
|
|
test_done
|