Mercurial > hg
changeset 33644:943c91326b23 stable 4.2.3
ssh: unban the use of pipe character in user@host:port string
This vulnerability was fixed by the previous patch and there were more ways
to exploit than using '|shellcmd'. So it doesn't make sense to reject only
pipe character.
Test cases are updated to actually try to exploit the bug. As the SSH bridge
of git/svn subrepos are not managed by our code, the tests for non-hg subrepos
are just removed.
This may be folded into the original patches.
| author | Yuya Nishihara <yuya@tcha.org> |
|---|---|
| date | Mon, 07 Aug 2017 22:22:28 +0900 |
| parents | 00a75672a9cb |
| children | fef451e3d9ca |
| files | mercurial/util.py tests/test-clone.t tests/test-pull.t tests/test-push.t tests/test-subrepo-git.t tests/test-subrepo-svn.t tests/test-subrepo.t |
| diffstat | 7 files changed, 42 insertions(+), 69 deletions(-) [+] |
line wrap: on
line diff
--- a/mercurial/util.py Fri Aug 04 23:54:12 2017 -0700 +++ b/mercurial/util.py Mon Aug 07 22:22:28 2017 +0900 @@ -2890,8 +2890,7 @@ Raises an error.Abort when the url is unsafe. """ path = urlreq.unquote(path) - if (path.startswith('ssh://-') or path.startswith('svn+ssh://-') - or '|' in path): + if path.startswith('ssh://-') or path.startswith('svn+ssh://-'): raise error.Abort(_('potentially unsafe url: %r') % (path,))
--- a/tests/test-clone.t Fri Aug 04 23:54:12 2017 -0700 +++ b/tests/test-clone.t Mon Aug 07 22:22:28 2017 +0900 @@ -1106,11 +1106,11 @@ $ hg clone 'ssh://%2DoProxyCommand=touch${IFS}owned/path' abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path' [255] - $ hg clone 'ssh://fakehost|shellcommand/path' - abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path' + $ hg clone 'ssh://fakehost|touch%20owned/path' + abort: no suitable response from remote hg! [255] - $ hg clone 'ssh://fakehost%7Cshellcommand/path' - abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path' + $ hg clone 'ssh://fakehost%7Ctouch%20owned/path' + abort: no suitable response from remote hg! [255] $ hg clone 'ssh://-oProxyCommand=touch owned%20foo@example.com/nonexistent/path'
--- a/tests/test-pull.t Fri Aug 04 23:54:12 2017 -0700 +++ b/tests/test-pull.t Mon Aug 07 22:22:28 2017 +0900 @@ -107,6 +107,11 @@ SEC: check for unsafe ssh url + $ cat >> $HGRCPATH << EOF + > [ui] + > ssh = sh -c "read l; read l; read l" + > EOF + $ hg pull 'ssh://-oProxyCommand=touch${IFS}owned/path' pulling from ssh://-oProxyCommand%3Dtouch%24%7BIFS%7Downed/path abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path' @@ -115,13 +120,15 @@ pulling from ssh://-oProxyCommand%3Dtouch%24%7BIFS%7Downed/path abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path' [255] - $ hg pull 'ssh://fakehost|shellcommand/path' - pulling from ssh://fakehost%7Cshellcommand/path - abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path' + $ hg pull 'ssh://fakehost|touch${IFS}owned/path' + pulling from ssh://fakehost%7Ctouch%24%7BIFS%7Downed/path + abort: no suitable response from remote hg! [255] - $ hg pull 'ssh://fakehost%7Cshellcommand/path' - pulling from ssh://fakehost%7Cshellcommand/path - abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path' + $ hg pull 'ssh://fakehost%7Ctouch%20owned/path' + pulling from ssh://fakehost%7Ctouch%20owned/path + abort: no suitable response from remote hg! [255] + $ [ ! -f owned ] || echo 'you got owned' + $ cd ..
--- a/tests/test-push.t Fri Aug 04 23:54:12 2017 -0700 +++ b/tests/test-push.t Mon Aug 07 22:22:28 2017 +0900 @@ -299,6 +299,11 @@ SEC: check for unsafe ssh url + $ cat >> $HGRCPATH << EOF + > [ui] + > ssh = sh -c "read l; read l; read l" + > EOF + $ hg -R test-revflag push 'ssh://-oProxyCommand=touch${IFS}owned/path' pushing to ssh://-oProxyCommand%3Dtouch%24%7BIFS%7Downed/path abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path' @@ -307,11 +312,13 @@ pushing to ssh://-oProxyCommand%3Dtouch%24%7BIFS%7Downed/path abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path' [255] - $ hg -R test-revflag push 'ssh://fakehost|shellcommand/path' - pushing to ssh://fakehost%7Cshellcommand/path - abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path' + $ hg -R test-revflag push 'ssh://fakehost|touch${IFS}owned/path' + pushing to ssh://fakehost%7Ctouch%24%7BIFS%7Downed/path + abort: no suitable response from remote hg! [255] - $ hg -R test-revflag push 'ssh://fakehost%7Cshellcommand/path' - pushing to ssh://fakehost%7Cshellcommand/path - abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path' + $ hg -R test-revflag push 'ssh://fakehost%7Ctouch%20owned/path' + pushing to ssh://fakehost%7Ctouch%20owned/path + abort: no suitable response from remote hg! [255] + + $ [ ! -f owned ] || echo 'you got owned'
--- a/tests/test-subrepo-git.t Fri Aug 04 23:54:12 2017 -0700 +++ b/tests/test-subrepo-git.t Mon Aug 07 22:22:28 2017 +0900 @@ -1205,26 +1205,3 @@ abort: potentially unsafe url: 'ssh://-oProxyCommand=rm${IFS}non-existent/path' (in subrepo s) [255] -also check for a pipe - - $ cd malicious-proxycommand - $ echo 's = [git]ssh://fakehost|shell/path' > .hgsub - $ hg ci -m 'change url to pipe' - $ cd .. - $ rm -r malicious-proxycommand-clone - $ hg clone malicious-proxycommand malicious-proxycommand-clone - updating to branch default - abort: potentially unsafe url: 'ssh://fakehost|shell/path' (in subrepo s) - [255] - -also check that a percent encoded '|' (%7C) doesn't work - - $ cd malicious-proxycommand - $ echo 's = [git]ssh://fakehost%7Cshell/path' > .hgsub - $ hg ci -m 'change url to percent encoded' - $ cd .. - $ rm -r malicious-proxycommand-clone - $ hg clone malicious-proxycommand malicious-proxycommand-clone - updating to branch default - abort: potentially unsafe url: 'ssh://fakehost|shell/path' (in subrepo s) - [255]
--- a/tests/test-subrepo-svn.t Fri Aug 04 23:54:12 2017 -0700 +++ b/tests/test-subrepo-svn.t Mon Aug 07 22:22:28 2017 +0900 @@ -668,30 +668,6 @@ abort: potentially unsafe url: 'svn+ssh://-oProxyCommand=touch owned nested' (in subrepo s) [255] -also check for a pipe - - $ cd ssh-vuln - $ echo "s = [svn]svn+ssh://fakehost|sh%20nested" > .hgsub - $ hg ci -m3 - $ cd .. - $ rm -r ssh-vuln-clone - $ hg clone ssh-vuln ssh-vuln-clone - updating to branch default - abort: potentially unsafe url: 'svn+ssh://fakehost|sh nested' (in subrepo s) - [255] - -also check that a percent encoded '|' (%7C) doesn't work - - $ cd ssh-vuln - $ echo "s = [svn]svn+ssh://fakehost%7Csh%20nested" > .hgsub - $ hg ci -m3 - $ cd .. - $ rm -r ssh-vuln-clone - $ hg clone ssh-vuln ssh-vuln-clone - updating to branch default - abort: potentially unsafe url: 'svn+ssh://fakehost|sh nested' (in subrepo s) - [255] - also check that hiding the attack in the username doesn't work: $ cd ssh-vuln
--- a/tests/test-subrepo.t Fri Aug 04 23:54:12 2017 -0700 +++ b/tests/test-subrepo.t Mon Aug 07 22:22:28 2017 +0900 @@ -1780,6 +1780,11 @@ test for ssh exploit 2017-07-25 + $ cat >> $HGRCPATH << EOF + > [ui] + > ssh = sh -c "read l; read l; read l" + > EOF + $ hg init malicious-proxycommand $ cd malicious-proxycommand $ echo 's = [hg]ssh://-oProxyCommand=touch${IFS}owned/path' > .hgsub @@ -1813,26 +1818,28 @@ also check for a pipe $ cd malicious-proxycommand - $ echo 's = [hg]ssh://fakehost|shell/path' > .hgsub + $ echo 's = [hg]ssh://fakehost|touch${IFS}owned/path' > .hgsub $ hg ci -m 'change url to pipe' $ cd .. $ rm -r malicious-proxycommand-clone $ hg clone malicious-proxycommand malicious-proxycommand-clone updating to branch default - abort: potentially unsafe url: 'ssh://fakehost|shell/path' (in subrepo s) + abort: no suitable response from remote hg! [255] + $ [ ! -f owned ] || echo 'you got owned' also check that a percent encoded '|' (%7C) doesn't work $ cd malicious-proxycommand - $ echo 's = [hg]ssh://fakehost%7Cshell/path' > .hgsub + $ echo 's = [hg]ssh://fakehost%7Ctouch%20owned/path' > .hgsub $ hg ci -m 'change url to percent encoded pipe' $ cd .. $ rm -r malicious-proxycommand-clone $ hg clone malicious-proxycommand malicious-proxycommand-clone updating to branch default - abort: potentially unsafe url: 'ssh://fakehost|shell/path' (in subrepo s) + abort: no suitable response from remote hg! [255] + $ [ ! -f owned ] || echo 'you got owned' and bad usernames: $ cd malicious-proxycommand