Mercurial > hg
changeset 33641:173ecccb9ee7 stable
subrepo: add tests for svn rogue ssh urls (SEC)
'ssh://' has an exploit that will pass the url blindly to the ssh
command, allowing a malicious person to have a subrepo with
'-oProxyCommand' which could run arbitrary code on a user's machine. In
addition, at least on Windows, a pipe '|' is able to execute arbitrary
commands.
When this happens, let's throw a big abort into the user's face so that
they can inspect what's going on.
| author | Sean Farley <sean@farley.io> |
|---|---|
| date | Mon, 31 Jul 2017 16:44:17 -0700 |
| parents | 55681baf4cf9 |
| children | ca398a50ca00 |
| files | mercurial/subrepo.py mercurial/util.py tests/test-subrepo-svn.t |
| diffstat | 3 files changed, 70 insertions(+), 1 deletions(-) [+] |
line wrap: on
line diff
--- a/mercurial/subrepo.py Mon Jul 31 16:04:44 2017 -0700 +++ b/mercurial/subrepo.py Mon Jul 31 16:44:17 2017 -0700 @@ -1274,6 +1274,10 @@ # The revision must be specified at the end of the URL to properly # update to a directory which has since been deleted and recreated. args.append('%s@%s' % (state[0], state[1])) + + # SEC: check that the ssh url is safe + util.checksafessh(state[0]) + status, err = self._svncommand(args, failok=True) _sanitize(self.ui, self.wvfs, '.svn') if not re.search('Checked out revision [0-9]+.', status):
--- a/mercurial/util.py Mon Jul 31 16:04:44 2017 -0700 +++ b/mercurial/util.py Mon Jul 31 16:44:17 2017 -0700 @@ -2890,7 +2890,8 @@ Raises an error.Abort when the url is unsafe. """ path = urlreq.unquote(path) - if path.startswith('ssh://-') or '|' in path: + if (path.startswith('ssh://-') or path.startswith('svn+ssh://-') + or '|' in path): raise error.Abort(_('potentially unsafe url: %r') % (path,))
--- a/tests/test-subrepo-svn.t Mon Jul 31 16:04:44 2017 -0700 +++ b/tests/test-subrepo-svn.t Mon Jul 31 16:44:17 2017 -0700 @@ -639,3 +639,67 @@ $ hg update -q -C '.^1' $ cd ../.. + +SEC: test for ssh exploit + + $ hg init ssh-vuln + $ cd ssh-vuln + $ echo "s = [svn]$SVNREPOURL/src" >> .hgsub + $ svn co --quiet "$SVNREPOURL"/src s + $ hg add .hgsub + $ hg ci -m1 + $ echo "s = [svn]svn+ssh://-oProxyCommand=touch%20owned%20nested" > .hgsub + $ hg ci -m2 + $ cd .. + $ hg clone ssh-vuln ssh-vuln-clone + updating to branch default + abort: potentially unsafe url: 'svn+ssh://-oProxyCommand=touch owned nested' (in subrepo s) + [255] + +also check that a percent encoded '-' (%2D) doesn't work + + $ cd ssh-vuln + $ echo "s = [svn]svn+ssh://%2DoProxyCommand=touch%20owned%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://-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 + $ echo "s = [svn]svn+ssh://%2DoProxyCommand=touch%20owned%20foo@example.com/nested" > .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://-oProxyCommand=touch owned foo@example.com/nested' (in subrepo s) + [255]