Skip to content

Commit

Permalink
[1.6.x] Added additional checks in is_safe_url to account for flexibl…
Browse files Browse the repository at this point in the history
…e parsing.

This is a security fix. Disclosure following shortly.
  • Loading branch information
mxsasha authored and apollo13 committed May 14, 2014
1 parent 1abcf3a commit 6011075
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 4 deletions.
12 changes: 8 additions & 4 deletions django/contrib/auth/tests/test_views.py
Expand Up @@ -444,8 +444,10 @@ def test_security_check(self, password='password'):

# 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")'):

Expand All @@ -467,8 +469,8 @@ def test_security_check(self, password='password'):
'/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' % {
Expand Down Expand Up @@ -660,8 +662,10 @@ def test_security_check(self, password='password'):

# 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' % {
Expand All @@ -681,8 +685,8 @@ def test_security_check(self, password='password'):
'/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' % {
Expand Down
12 changes: 12 additions & 0 deletions django/utils/http.py
Expand Up @@ -256,6 +256,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'])
29 changes: 29 additions & 0 deletions tests/utils_tests/test_http.py
Expand Up @@ -91,6 +91,35 @@ def test_base36(self):
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):
Expand Down

0 comments on commit 6011075

Please sign in to comment.