mirror of
https://github.com/facebook/sapling.git
synced 2024-10-07 23:38:50 +03:00
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.
This commit is contained in:
parent
f904aef7aa
commit
da301ac6a0
@ -1281,6 +1281,10 @@ class svnsubrepo(abstractsubrepo):
|
||||
# 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):
|
||||
|
@ -2905,7 +2905,8 @@ def checksafessh(path):
|
||||
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,))
|
||||
|
||||
|
@ -639,3 +639,67 @@ Test that sanitizing is omitted in meta data area:
|
||||
$ 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 subrepository "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 subrepository "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 subrepository "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 subrepository "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 subrepository "s")
|
||||
[255]
|
||||
|
Loading…
Reference in New Issue
Block a user