Mercurial > hg

changeset 33659:8cb9e921ef8c stable

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
ssh: quote parameters using shellquote (SEC) This patch uses shellquote to quote ssh parameters more strictly to avoid shell injection.
author Jun Wu <quark@fb.com>
date Fri, 04 Aug 2017 23:54:12 -0700
parents db83a1df03fe
children 3fee7f7d2da0
files mercurial/posix.py mercurial/sshpeer.py mercurial/windows.py tests/test-clone.t tests/test-ssh-bundle1.t tests/test-ssh.t
diffstat 6 files changed, 53 insertions(+), 9 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/posix.py	Mon Jul 31 14:55:11 2017 -0700
+++ b/mercurial/posix.py	Fri Aug 04 23:54:12 2017 -0700
@@ -92,10 +92,13 @@
 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"""
--- a/mercurial/sshpeer.py	Mon Jul 31 14:55:11 2017 -0700
+++ b/mercurial/sshpeer.py	Fri Aug 04 23:54:12 2017 -0700
@@ -151,10 +151,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,
--- a/mercurial/windows.py	Mon Jul 31 14:55:11 2017 -0700
+++ b/mercurial/windows.py	Fri Aug 04 23:54:12 2017 -0700
@@ -208,7 +208,10 @@
         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
--- a/tests/test-clone.t	Mon Jul 31 14:55:11 2017 -0700
+++ b/tests/test-clone.t	Fri Aug 04 23:54:12 2017 -0700
@@ -1100,6 +1100,11 @@
 
 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 @@
   $ 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
--- a/tests/test-ssh-bundle1.t	Mon Jul 31 14:55:11 2017 -0700
+++ b/tests/test-ssh-bundle1.t	Fri Aug 04 23:54:12 2017 -0700
@@ -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
--- a/tests/test-ssh.t	Mon Jul 31 14:55:11 2017 -0700
+++ b/tests/test-ssh.t	Fri Aug 04 23:54:12 2017 -0700
@@ -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