Commit 7feb54bb authored by Tim Graham's avatar Tim Graham
Browse files

[1.4.x] Added additional checks in is_safe_url to account for flexible parsing.

This is a security fix. Disclosure following shortly.
parent 28e23306
Loading
Loading
Loading
Loading
+8 −4
Original line number Diff line number Diff line
@@ -307,8 +307,10 @@ class LoginTest(AuthViewsTestCase):

        # 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 @@ class LoginTest(AuthViewsTestCase):
                         '/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 @@ class LogoutTest(AuthViewsTestCase):

        # 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 @@ class LogoutTest(AuthViewsTestCase):
                         '/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' % {
+12 −0
Original line number Diff line number Diff line
@@ -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.
    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'])
+30 −0
Original line number Diff line number Diff line
@@ -78,3 +78,33 @@ class TestUtilsHttp(unittest.TestCase):
        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)