# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 2017-08-05 06:54:12
# Node ID 8cb9e921ef8c4ffa388bc78ef49ac6f997b960a0
# Parent  db83a1df03fe6f131773eb1813dd7ccd222f89a8

ssh: quote parameters using shellquote (SEC)

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

diff --git a/mercurial/posix.py b/mercurial/posix.py
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -92,10 +92,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
-    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"""
diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -151,10 +151,7 @@ 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,
diff --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -208,7 +208,10 @@ 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
diff --git a/tests/test-clone.t b/tests/test-clone.t
--- a/tests/test-clone.t
+++ b/tests/test-clone.t
@@ -1100,6 +1100,11 @@ 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 @@ SEC: check for unsafe ssh url
   $ 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-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 @@ 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
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 @@ 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