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

@@ -2890,8 +2890,7 b' def checksafessh(path):'
     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,))
 

tests/test-clone.t

@@ -1106,11 +1106,11 b' SEC: check for unsafe ssh url'
   $ 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 [e-mail]/nonexistent/path'

tests/test-pull.t

@@ -107,6 +107,11 b' regular shell commands.'
 
 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 b' SEC: check for unsafe ssh url'
   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 ..

tests/test-push.t

@@ -299,6 +299,11 b' Test push hook locking'
 
 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 b' SEC: check for unsafe ssh url'
   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'

tests/test-subrepo-git.t

@@ -1205,26 +1205,3 b" also check that a percent encoded '-' (%"
   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]

tests/test-subrepo-svn.t

@@ -668,30 +668,6 b" also check that a percent encoded '-' (%"
   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

tests/test-subrepo.t

@@ -1780,6 +1780,11 b" Test that '[paths]' is configured correc"
 
 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 b" also check that a percent encoded '-' (%"
 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