Added additional checks in is_safe_url to account for flexible parsing.
This is a security fix. Disclosure following shortly.
This commit is contained in:
parent
3800f63721
commit
255449c1ee
|
@ -482,8 +482,10 @@ class LoginTest(AuthViewsTestCase):
|
||||||
|
|
||||||
# Those URLs should not pass the security check
|
# Those URLs should not pass the security check
|
||||||
for bad_url in ('http://example.com',
|
for bad_url in ('http://example.com',
|
||||||
|
'http:///example.com',
|
||||||
'https://example.com',
|
'https://example.com',
|
||||||
'ftp://exampel.com',
|
'ftp://exampel.com',
|
||||||
|
'///example.com',
|
||||||
'//example.com',
|
'//example.com',
|
||||||
'javascript:alert("XSS")'):
|
'javascript:alert("XSS")'):
|
||||||
|
|
||||||
|
@ -505,8 +507,8 @@ class LoginTest(AuthViewsTestCase):
|
||||||
'/view/?param=https://example.com',
|
'/view/?param=https://example.com',
|
||||||
'/view?param=ftp://exampel.com',
|
'/view?param=ftp://exampel.com',
|
||||||
'view/?param=//example.com',
|
'view/?param=//example.com',
|
||||||
'https:///',
|
'https://testserver/',
|
||||||
'HTTPS:///',
|
'HTTPS://testserver/',
|
||||||
'//testserver/',
|
'//testserver/',
|
||||||
'/url%20with%20spaces/'): # see ticket #12534
|
'/url%20with%20spaces/'): # see ticket #12534
|
||||||
safe_url = '%(url)s?%(next)s=%(good_url)s' % {
|
safe_url = '%(url)s?%(next)s=%(good_url)s' % {
|
||||||
|
@ -743,8 +745,10 @@ class LogoutTest(AuthViewsTestCase):
|
||||||
|
|
||||||
# Those URLs should not pass the security check
|
# Those URLs should not pass the security check
|
||||||
for bad_url in ('http://example.com',
|
for bad_url in ('http://example.com',
|
||||||
|
'http:///example.com',
|
||||||
'https://example.com',
|
'https://example.com',
|
||||||
'ftp://exampel.com',
|
'ftp://exampel.com',
|
||||||
|
'///example.com',
|
||||||
'//example.com',
|
'//example.com',
|
||||||
'javascript:alert("XSS")'):
|
'javascript:alert("XSS")'):
|
||||||
nasty_url = '%(url)s?%(next)s=%(bad_url)s' % {
|
nasty_url = '%(url)s?%(next)s=%(bad_url)s' % {
|
||||||
|
@ -764,8 +768,8 @@ class LogoutTest(AuthViewsTestCase):
|
||||||
'/view/?param=https://example.com',
|
'/view/?param=https://example.com',
|
||||||
'/view?param=ftp://exampel.com',
|
'/view?param=ftp://exampel.com',
|
||||||
'view/?param=//example.com',
|
'view/?param=//example.com',
|
||||||
'https:///',
|
'https://testserver/',
|
||||||
'HTTPS:///',
|
'HTTPS://testserver/',
|
||||||
'//testserver/',
|
'//testserver/',
|
||||||
'/url%20with%20spaces/'): # see ticket #12534
|
'/url%20with%20spaces/'): # see ticket #12534
|
||||||
safe_url = '%(url)s?%(next)s=%(good_url)s' % {
|
safe_url = '%(url)s?%(next)s=%(good_url)s' % {
|
||||||
|
|
|
@ -272,6 +272,18 @@ def is_safe_url(url, host=None):
|
||||||
"""
|
"""
|
||||||
if not url:
|
if not url:
|
||||||
return False
|
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)
|
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
|
return ((not url_info.netloc or url_info.netloc == host) and
|
||||||
(not url_info.scheme or url_info.scheme in ['http', 'https']))
|
(not url_info.scheme or url_info.scheme in ['http', 'https']))
|
||||||
|
|
|
@ -89,6 +89,36 @@ class TestUtilsHttp(unittest.TestCase):
|
||||||
self.assertEqual(http.int_to_base36(n), b36)
|
self.assertEqual(http.int_to_base36(n), b36)
|
||||||
self.assertEqual(http.base36_to_int(b36), n)
|
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):
|
class ETagProcessingTests(unittest.TestCase):
|
||||||
def testParsing(self):
|
def testParsing(self):
|
||||||
|
|
Loading…
Reference in New Issue