ssh: quote parameters using shellquote (SEC)
This patch uses shellquote to quote ssh parameters more strictly to avoid
shell injection.
--- 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