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

Commit ad32c21

Browse files
mxsashatimgraham
mxsasha
authored and
timgraham
committed
[1.5.x] Added additional checks in is_safe_url to account for flexible parsing.
This is a security fix. Disclosure following shortly.
1 parent 4001ec8 commit ad32c21

File tree

3 files changed

+49
-4
lines changed

3 files changed

+49
-4
lines changed

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

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

327327
# Those URLs should not pass the security check
328328
for bad_url in ('http://example.com',
329+
'http:///example.com',
329330
'https://example.com',
330331
'ftp://exampel.com',
332+
'///example.com',
331333
'//example.com',
332334
'javascript:alert("XSS")'):
333335

@@ -349,8 +351,8 @@ def test_security_check(self, password='password'):
349351
'/view/?param=https://example.com',
350352
'/view?param=ftp://exampel.com',
351353
'view/?param=//example.com',
352-
'https:///',
353-
'HTTPS:///',
354+
'https://testserver/',
355+
'HTTPS://testserver/',
354356
'//testserver/',
355357
'/url%20with%20spaces/'): # see ticket #12534
356358
safe_url = '%(url)s?%(next)s=%(good_url)s' % {
@@ -521,8 +523,10 @@ def test_security_check(self, password='password'):
521523

522524
# Those URLs should not pass the security check
523525
for bad_url in ('http://example.com',
526+
'http:///example.com',
524527
'https://example.com',
525528
'ftp://exampel.com',
529+
'///example.com',
526530
'//example.com',
527531
'javascript:alert("XSS")'):
528532
nasty_url = '%(url)s?%(next)s=%(bad_url)s' % {
@@ -542,8 +546,8 @@ def test_security_check(self, password='password'):
542546
'/view/?param=https://example.com',
543547
'/view?param=ftp://exampel.com',
544548
'view/?param=//example.com',
545-
'https:///',
546-
'HTTPS:///',
549+
'https://testserver/',
550+
'HTTPS://testserver/',
547551
'//testserver/',
548552
'/url%20with%20spaces/'): # see ticket #12534
549553
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
@@ -237,6 +237,18 @@ def is_safe_url(url, host=None):
237237
"""
238238
if not url:
239239
return False
240+
# Chrome treats \ completely as /
241+
url = url.replace('\\', '/')
242+
# Chrome considers any URL with more than two slashes to be absolute, but
243+
# urlaprse is not so flexible. Treat any url with three slashes as unsafe.
244+
if url.startswith('///'):
245+
return False
240246
url_info = urllib_parse.urlparse(url)
247+
# Forbid URLs like http:///example.com - with a scheme, but without a hostname.
248+
# In that URL, example.com is not the hostname but, a path component. However,
249+
# Chrome will still consider example.com to be the hostname, so we must not
250+
# allow this syntax.
251+
if not url_info.netloc and url_info.scheme:
252+
return False
241253
return (not url_info.netloc or url_info.netloc == host) and \
242254
(not url_info.scheme or url_info.scheme in ['http', 'https'])

‎tests/regressiontests/utils/http.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,35 @@ def test_base36(self):
9191
self.assertEqual(http.int_to_base36(n), b36)
9292
self.assertEqual(http.base36_to_int(b36), n)
9393

94+
def test_is_safe_url(self):
95+
for bad_url in ('http://example.com',
96+
'http:///example.com',
97+
'https://example.com',
98+
'ftp://exampel.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+
r'\/example.com',
107+
r'/\example.com',
108+
'http:///example.com',
109+
'http:/\//example.com',
110+
'http:\/example.com',
111+
'http:/\example.com',
112+
'javascript:alert("XSS")'):
113+
self.assertFalse(http.is_safe_url(bad_url, host='testserver'), "%s should be blocked" % bad_url)
114+
for good_url in ('/view/?param=http://example.com',
115+
'/view/?param=https://example.com',
116+
'/view?param=ftp://exampel.com',
117+
'view/?param=//example.com',
118+
'https://testserver/',
119+
'HTTPS://testserver/',
120+
'//testserver/',
121+
'/url%20with%20spaces/'):
122+
self.assertTrue(http.is_safe_url(good_url, host='testserver'), "%s should be allowed" % good_url)
94123

95124
class ETagProcessingTests(unittest.TestCase):
96125
def testParsing(self):

0 commit comments

Comments
 (0)
0