Skip to content
Browse files

[1.4.x] Added additional checks in is_safe_url to account for flexibl…

…e parsing.

This is a security fix. Disclosure following shortly.
  • Loading branch information...
1 parent 28e2330 commit 7feb54bbae3f637ab3c4dd4831d4385964f574df @timgraham timgraham committed May 12, 2014
Showing with 50 additions and 4 deletions.
  1. +8 −4 django/contrib/auth/tests/views.py
  2. +12 −0 django/utils/http.py
  3. +30 −0 tests/regressiontests/utils/http.py
View
12 django/contrib/auth/tests/views.py
@@ -307,8 +307,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")'):
@@ -330,8 +332,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' % {
@@ -467,8 +469,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' % {
@@ -488,8 +492,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' % {
View
12 django/utils/http.py
@@ -234,6 +234,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.
@AndreaCrotti
AndreaCrotti added a note Jul 5, 2014

Small typo, urlaprse -> urlparse

@timgraham
Django member
timgraham added a note Jul 5, 2014

It's been fixed in master, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if url.startswith('///'):
+ return False
url_info = urlparse.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[1] and url_info[0]:
+ return False
return (not url_info[1] or url_info[1] == host) and \
(not url_info[0] or url_info[0] in ['http', 'https'])
View
30 tests/regressiontests/utils/http.py
@@ -78,3 +78,33 @@ def test_base36(self):
for n, b36 in [(0, '0'), (1, '1'), (42, '16'), (818469960, 'django')]:
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)

0 comments on commit 7feb54b

Please sign in to comment.
Something went wrong with that request. Please try again.