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.
--- a/mercurial/util.py Fri Aug 04 23:54:12 2017 -0700
+++ b/mercurial/util.py Mon Aug 07 22:22:28 2017 +0900
@@ -2905,8 +2905,7 @@
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,))
--- a/tests/test-clone.t Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-clone.t Mon Aug 07 22:22:28 2017 +0900
@@ -1111,11 +1111,11 @@
$ 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 owned%20foo@example.com/nonexistent/path'
--- a/tests/test-pull.t Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-pull.t Mon Aug 07 22:22:28 2017 +0900
@@ -107,6 +107,11 @@
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 @@
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 ..
--- a/tests/test-push.t Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-push.t Mon Aug 07 22:22:28 2017 +0900
@@ -319,6 +319,11 @@
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'
@@ -327,11 +332,13 @@
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'
--- a/tests/test-subrepo-git.t Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-subrepo-git.t Mon Aug 07 22:22:28 2017 +0900
@@ -1213,27 +1213,3 @@
updating to branch default
abort: potentially unsafe url: 'ssh://-oProxyCommand=rm${IFS}non-existent/path' (in subrepository "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 subrepository "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 subrepository "s")
- [255]
--- a/tests/test-subrepo-svn.t Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-subrepo-svn.t Mon Aug 07 22:22:28 2017 +0900
@@ -668,30 +668,6 @@
abort: potentially unsafe url: 'svn+ssh://-oProxyCommand=touch owned nested' (in subrepository "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 subrepository "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 subrepository "s")
- [255]
-
also check that hiding the attack in the username doesn't work:
$ cd ssh-vuln
--- a/tests/test-subrepo.t Fri Aug 04 23:54:12 2017 -0700
+++ b/tests/test-subrepo.t Mon Aug 07 22:22:28 2017 +0900
@@ -1792,6 +1792,11 @@
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
@@ -1825,26 +1830,28 @@
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 subrepository "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 subrepository "s")
+ abort: no suitable response from remote hg!
[255]
+ $ [ ! -f owned ] || echo 'you got owned'
and bad usernames:
$ cd malicious-proxycommand