8000 [1.7.x] Added additional checks in is_safe_url to account for flexibl… · django/django@e7b0cac · GitHub
[go: up one dir, main page]
Skip to content

Commit e7b0cac

Browse files
mxsashaapollo13
mxsasha
authored and
apollo13
committed
[1.7.x] Added additional checks in is_safe_url to account for flexible parsing.
This is a security fix. Disclosure following shortly.
1 parent 7fef18b commit e7b0cac

File tree

3 files changed

+50
-4
lines changed

3 files changed

+50
-4
lines changed

‎django/contrib/auth/tests/test_views.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,10 @@ def test_security_check(self, password='password'):
483483

484484
# Those URLs should not pass the security check
485485
for bad_url in ('http://example.com',
486+
'http:///example.com',
486487
'https://example.com',
487488
'ftp://exampel.com',
489+
'///example.com',
488490
'//example.com',
489491
'javascript:alert("XSS")'):
490492

@@ -506,8 +508,8 @@ def test_security_check(self, password='password'):
506508
'/view/?param=https://example.com',
507509
'/view?param=ftp://exampel.com',
508510
'view/?param=//example.com',
509-
'https:///',
510-
'HTTPS:///',
511+
'https://testserver/',
512+
'HTTPS://testserver/',
511513
'//testserver/',
512514
'/url%20with%20spaces/'): # see ticket #12534
513515
safe_url = '%(url)s?%(next)s=%(good_url)s' % {
@@ -744,8 +746,10 @@ def test_security_check(self, password='password'):
744746

745747
# Those URLs should not pass the security check
746748
for bad_url in ('http://example.com',
749+
'http:///example.com',
747750
'https://example.com',
748751
'ftp://exampel.com',
752+
'///example.com',
749753
'//example.com',
750754
'javascript:alert("XSS")'):
751755
nasty_url = '%(url)s?%(next)s=%(bad_url)s' % {
@@ -765,8 +769,8 @@ def test_security_check(self, password='password'):
765769
'/view/?param=https://example.com',
766770
'/view?param=ftp://exampel.com',
767771
'view/?param=//example.com',
768-
'https:///',
769-
'HTTPS:///',
772+
'https://testserver/',
773+
'HTTPS://testserver/',
770774
'//testserver/',
771775
'/url%20with%20spaces/'): # see ticket #12534
772776
safe_url = '%(url)s?%(next)s=%(good_url)s' % {

‎django/utils/http.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,18 @@ def is_safe_url(url, host=None):
272272
"""
273273
if not url:
274274
return False
275+
# Chrome treats \ completely as /
276+
url = url.replace('\\', '/')
277+
# Chrome considers any URL with more than two slashes to be absolute, but
278+
# urlaprse is not so flexible. Treat any url with three slashes as unsafe.
279+
if url.startswith('///'):
280+
return False
275281
url_info = urlparse(url)
282+
# Forbid URLs like http:///example.com - with a scheme, but without a hostname.
283+
# In that URL, example.com is not the hostname but, a path component. However,
284+
# Chrome will still consider example.com to be the hostname, so we must not
285+
# allow this syntax.
286+
if not url_info.netloc and url_info.scheme:
287+
return False
276288
return ((not url_info.netloc or url_info.netloc == host) and
277289
(not url_info.scheme or url_info.scheme in ['http', 'https']))

‎tests/utils_tests/test_http.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,36 @@ def test_base36(self):
8989
self.assertEqual(http.int_to_base36(n), b36)
9090
self.assertEqual(http.base36_to_int(b36), n)
9191

92+
def test_is_safe_url(self):
93+
for bad_url in ('http://example.com',
94+
'http:///example.com',
95+
'https://example.com',
96+
'ftp://exampel.com',
97+
r'\\example.com',
98+
r'\\\example.com',
99+
r'/\\/example.com',
100+
r'\\\example.com',
101+
r'\\example.com',
102+
r'\\//example.com',
103+
r'/\/example.com',
104+
r'\/example.com',
105+
r'/\example.com',
106+
'http:///example.com',
107+
'http:/\//example.com',
108+
'http:\/example.com',
109+
'http:/\example.com',
110+
'javascript:alert("XSS")'):
111+
self.assertFalse(http.is_safe_url(bad_url, host='testserver'), "%s should be blocked" % bad_url)
112+
for good_url in ('/view/?param=http://example.com',
113+
'/view/?param=https://example.com',
114+
'/view?param=ftp://exampel.com',
115+
'view/?param=//example.com',
116+
'https://testserver/',
117+
'HTTPS://testserver/',
118+
'//testserver/',
119+
'/url%20with%20spaces/'):
120+
self.assertTrue(http.is_safe_url(good_url, host='testserver'), "%s should be allowed" % good_url)
121+
92122

93123
class ETagProcessingTests(unittest.TestCase):
94124
def testParsing(self):

0 commit comments

Comments
 (0)
0