Mercurial Mercurial > mercurial / changeset
summary | shortlog | changelog | graph | tags | bookmarks | branches | files | changeset | raw | help
Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
ssh: unban the use of pipe character in user@host:port string stable 4.2.3
authorYuya Nishihara <yuya@tcha.org>
Mon, 07 Aug 2017 22:22:28 +0900
branchstable
changeset 33644 943c91326b23
parent 33643 00a75672a9cb
child 33645 fef451e3d9ca
ssh: unban the use of pipe character in user@host:port string This vulnerability was fixed by the previous patch and there were more ways to exploit than using '|shellcmd'. So it doesn't make sense to reject only pipe character. Test cases are updated to actually try to exploit the bug. As the SSH bridge of git/svn subrepos are not managed by our code, the tests for non-hg subrepos are just removed. This may be folded into the original patches.
mercurial/util.py file | annotate | diff | comparison | revisions
tests/test-clone.t file | annotate | diff | comparison | revisions
tests/test-pull.t file | annotate | diff | comparison | revisions
tests/test-push.t file | annotate | diff | comparison | revisions
tests/test-subrepo-git.t file | annotate | diff | comparison | revisions
tests/test-subrepo-svn.t file | annotate | diff | comparison | revisions
tests/test-subrepo.t file | annotate | diff | comparison | revisions 
--- a/mercurial/util.py	Fri Aug 04 23:54:12 2017 -0700
+++ b/mercurial/util.py	Mon Aug 07 22:22:28 2017 +0900
@@ -2890,8 +2890,7 @@
     Raises an error.Abort when the url is unsafe.
     """
     path = urlreq.unquote(path)
-    if (path.startswith('ssh://-') or path.startswith('svn+ssh://-')
-        or '|' in path):
+    if path.startswith('ssh://-') or path.startswith('svn+ssh://-'):
         raise error.Abort(_('potentially unsafe url: %r') %
                           (path,))
 
--- a/tests/test-clone.t	Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-clone.t	Mon Aug 07 22:22:28 2017 +0900
@@ -1106,11 +1106,11 @@
   $ hg clone 'ssh://%2DoProxyCommand=touch${IFS}owned/path'
   abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path'
   [255]
-  $ hg clone 'ssh://fakehost|shellcommand/path'
-  abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path'
+  $ hg clone 'ssh://fakehost|touch%20owned/path'
+  abort: no suitable response from remote hg!
   [255]
-  $ hg clone 'ssh://fakehost%7Cshellcommand/path'
-  abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path'
+  $ 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'
--- a/tests/test-pull.t	Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-pull.t	Mon Aug 07 22:22:28 2017 +0900
@@ -107,6 +107,11 @@
 
 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'
@@ -115,13 +120,15 @@
   pulling from ssh://-oProxyCommand%3Dtouch%24%7BIFS%7Downed/path
   abort: potentially unsafe url: 'ssh://-oProxyCommand=touch${IFS}owned/path'
   [255]
-  $ hg pull 'ssh://fakehost|shellcommand/path'
-  pulling from ssh://fakehost%7Cshellcommand/path
-  abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path'
+  $ 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%7Cshellcommand/path'
-  pulling from ssh://fakehost%7Cshellcommand/path
-  abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path'
+  $ 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 ..
--- a/tests/test-push.t	Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-push.t	Mon Aug 07 22:22:28 2017 +0900
@@ -299,6 +299,11 @@
 
 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'
@@ -307,11 +312,13 @@
   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|shellcommand/path'
-  pushing to ssh://fakehost%7Cshellcommand/path
-  abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path'
+  $ 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%7Cshellcommand/path'
-  pushing to ssh://fakehost%7Cshellcommand/path
-  abort: potentially unsafe url: 'ssh://fakehost|shellcommand/path'
+  $ 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'
--- a/tests/test-subrepo-git.t	Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-subrepo-git.t	Mon Aug 07 22:22:28 2017 +0900
@@ -1205,26 +1205,3 @@
   abort: potentially unsafe url: 'ssh://-oProxyCommand=rm${IFS}non-existent/path' (in subrepo s)
   [255]
 
-also check for a pipe
-
-  $ cd malicious-proxycommand
-  $ echo 's = [git]ssh://fakehost|shell/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: potentially unsafe url: 'ssh://fakehost|shell/path' (in subrepo s)
-  [255]
-
-also check that a percent encoded '|' (%7C) doesn't work
-
-  $ cd malicious-proxycommand
-  $ echo 's = [git]ssh://fakehost%7Cshell/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://fakehost|shell/path' (in subrepo s)
-  [255]
--- a/tests/test-subrepo-svn.t	Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-subrepo-svn.t	Mon Aug 07 22:22:28 2017 +0900
@@ -668,30 +668,6 @@
   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
--- a/tests/test-subrepo.t	Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-subrepo.t	Mon Aug 07 22:22:28 2017 +0900
@@ -1780,6 +1780,11 @@
 
 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
@@ -1813,26 +1818,28 @@
 also check for a pipe
 
   $ cd malicious-proxycommand
-  $ echo 's = [hg]ssh://fakehost|shell/path' > .hgsub
+  $ 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: potentially unsafe url: 'ssh://fakehost|shell/path' (in subrepo s)
+  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%7Cshell/path' > .hgsub
+  $ 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: potentially unsafe url: 'ssh://fakehost|shell/path' (in subrepo s)
+  abort: no suitable response from remote hg!
   [255]
+  $ [ ! -f owned ] || echo 'you got owned'
 
 and bad usernames:
   $ cd malicious-proxycommand
mercurial