ssh: quote parameters using shellquote (SEC)

This patch uses shellquote to quote ssh parameters more strictly to avoid
shell injection.

mercurial/posix.py

@@ -92,10 +92,13 b' def parsepatchoutput(output_line):'
 def sshargs(sshcmd, host, user, port):
     '''Build argument list for ssh'''
     args = user and ("%s@%s" % (user, host)) or host
-    if '-' in args[:2]:
+    if '-' in args[:1]:
         raise error.Abort(
             _('illegal ssh hostname or username starting with -: %s') % args)
-    return port and ("%s -p %s" % (args, port)) or args
+    args = shellquote(args)
+    if port:
+        args = '-p %s %s' % (shellquote(port), args)
+    return args
 
 def isexec(f):
     """check whether a file is executable"""

mercurial/sshpeer.py

@@ -151,10 +151,7 b' class sshpeer(wireproto.wirepeer):'
         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,

mercurial/windows.py

@@ -208,7 +208,10 b' def sshargs(sshcmd, host, user, port):'
         raise error.Abort(
             _('illegal ssh hostname or username starting with - or /: %s') %
             args)
-    return port and ("%s %s %s" % (args, pflag, port)) or args
+    args = shellquote(args)
+    if port:
+        args = '%s %s %s' % (pflag, shellquote(port), args)
+    return args
 
 def setflags(f, l, x):
     pass

tests/test-clone.t

@@ -1100,6 +1100,11 b' pooled".'
 
 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]
@@ -1116,6 +1121,42 b' SEC: check for unsafe ssh url'
   $ hg clone 'ssh://-oProxyCommand=touch [e-mail]/nonexistent/path'
   abort: potentially unsafe url: 'ssh://-oProxyCommand=touch owned [e-mail]/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

tests/test-ssh-bundle1.t

@@ -461,7 +461,7 b' debug output'
 
   $ 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

tests/test-ssh.t

@@ -477,7 +477,7 b' debug output'
 
   $ 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