summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorErik Romijn <eromijn@solidlinks.nl>2014-05-12 07:38:39 -0400
committerFlorian Apolloner <florian@apolloner.eu>2014-05-14 10:19:48 +0200
commit255449c1ee61c14778658caae8c430fa4d76afd6 (patch)
tree9c1a2b5deccc1af3961c77ecf27f41ffeb494c33
parent3800f63721f6fc1c940abfb10fe21257dcf13521 (diff)
Added additional checks in is_safe_url to account for flexible parsing.
This is a security fix. Disclosure following shortly.
-rw-r--r--django/contrib/auth/tests/test_views.py12
-rw-r--r--django/utils/http.py12
-rw-r--r--tests/utils_tests/test_http.py30
3 files changed, 50 insertions, 4 deletions
diff --git a/django/contrib/auth/tests/test_views.py b/django/contrib/auth/tests/test_views.py
index a5be9ffff1..dcfda61ca9 100644
--- a/django/contrib/auth/tests/test_views.py
+++ b/django/contrib/auth/tests/test_views.py
@@ -482,8 +482,10 @@ class LoginTest(AuthViewsTestCase):
# Those URLs should not pass the security check
for bad_url in ('http://example.com',
+ 'http:///example.com',
'https://example.com',
'ftp://exampel.com',
+ '///example.com',
'//example.com',
'javascript:alert("XSS")'):
@@ -505,8 +507,8 @@ class LoginTest(AuthViewsTestCase):
'/view/?param=https://example.com',
'/view?param=ftp://exampel.com',
'view/?param=//example.com',
- 'https:///',
- 'HTTPS:///',
+ 'https://testserver/',
+ 'HTTPS://testserver/',
'//testserver/',
'/url%20with%20spaces/'): # see ticket #12534
safe_url = '%(url)s?%(next)s=%(good_url)s' % {
@@ -743,8 +745,10 @@ class LogoutTest(AuthViewsTestCase):
# Those URLs should not pass the security check
for bad_url in ('http://example.com',
+ 'http:///example.com',
'https://example.com',
'ftp://exampel.com',
+ '///example.com',
'//example.com',
'javascript:alert("XSS")'):
nasty_url = '%(url)s?%(next)s=%(bad_url)s' % {
@@ -764,8 +768,8 @@ class LogoutTest(AuthViewsTestCase):
'/view/?param=https://example.com',
'/view?param=ftp://exampel.com',
'view/?param=//example.com',
- 'https:///',
- 'HTTPS:///',
+ 'https://testserver/',
+ 'HTTPS://testserver/',
'//testserver/',
'/url%20with%20spaces/'): # see ticket #12534
safe_url = '%(url)s?%(next)s=%(good_url)s' % {
diff --git a/django/utils/http.py b/django/utils/http.py
index e3365776f3..28245d23f4 100644
--- a/django/utils/http.py
+++ b/django/utils/http.py
@@ -272,6 +272,18 @@ def is_safe_url(url, host=None):
"""
if not url:
return False
+ # Chrome treats \ completely as /
+ url = url.replace('\\', '/')
+ # Chrome considers any URL with more than two slashes to be absolute, but
+ # urlaprse is not so flexible. Treat any url with three slashes as unsafe.
+ if url.startswith('///'):
+ return False
url_info = urlparse(url)
+ # Forbid URLs like http:///example.com - with a scheme, but without a hostname.
+ # In that URL, example.com is not the hostname but, a path component. However,
+ # Chrome will still consider example.com to be the hostname, so we must not
+ # allow this syntax.
+ if not url_info.netloc and url_info.scheme:
+ return False
return ((not url_info.netloc or url_info.netloc == host) and
(not url_info.scheme or url_info.scheme in ['http', 'https']))
diff --git a/tests/utils_tests/test_http.py b/tests/utils_tests/test_http.py
index edd0fde44f..eec439a11c 100644
--- a/tests/utils_tests/test_http.py
+++ b/tests/utils_tests/test_http.py
@@ -89,6 +89,36 @@ class TestUtilsHttp(unittest.TestCase):
self.assertEqual(http.int_to_base36(n), b36)
self.assertEqual(http.base36_to_int(b36), n)
+ def test_is_safe_url(self):
+ for bad_url in ('http://example.com',
+ 'http:///example.com',
+ 'https://example.com',
+ 'ftp://exampel.com',
+ r'\\example.com',
+ r'\\\example.com',
+ r'/\\/example.com',
+ r'\\\example.com',
+ r'\\example.com',
+ r'\\//example.com',
+ r'/\/example.com',
+ r'\/example.com',
+ r'/\example.com',
+ 'http:///example.com',
+ 'http:/\//example.com',
+ 'http:\/example.com',
+ 'http:/\example.com',
+ 'javascript:alert("XSS")'):
+ self.assertFalse(http.is_safe_url(bad_url, host='testserver'), "%s should be blocked" % bad_url)
+ for good_url in ('/view/?param=http://example.com',
+ '/view/?param=https://example.com',
+ '/view?param=ftp://exampel.com',
+ 'view/?param=//example.com',
+ 'https://testserver/',
+ 'HTTPS://testserver/',
+ '//testserver/',
+ '/url%20with%20spaces/'):
+ self.assertTrue(http.is_safe_url(good_url, host='testserver'), "%s should be allowed" % good_url)
+
class ETagProcessingTests(unittest.TestCase):
def testParsing(self):