[1.4.x] Made is_safe_url() reject URLs that start with control characters.

This is a security fix; disclosure to follow shortly.
This commit is contained in:
Tim Graham 2015-03-09 20:05:13 -04:00
parent 3b20558beb
commit 2342693b31
3 changed files with 30 additions and 2 deletions

View File

@ -4,6 +4,7 @@ import re
import sys import sys
import urllib import urllib
import urlparse import urlparse
import unicodedata
from email.utils import formatdate from email.utils import formatdate
from django.utils.datastructures import MultiValueDict from django.utils.datastructures import MultiValueDict
@ -232,9 +233,10 @@ def is_safe_url(url, host=None):
Always returns ``False`` on an empty url. Always returns ``False`` on an empty url.
""" """
if url is not None:
url = url.strip()
if not url: if not url:
return False return False
url = url.strip()
# Chrome treats \ completely as / # Chrome treats \ completely as /
url = url.replace('\\', '/') url = url.replace('\\', '/')
# Chrome considers any URL with more than two slashes to be absolute, but # Chrome considers any URL with more than two slashes to be absolute, but
@ -248,5 +250,10 @@ def is_safe_url(url, host=None):
# allow this syntax. # allow this syntax.
if not url_info[1] and url_info[0]: if not url_info[1] and url_info[0]:
return False return False
# Forbid URLs that start with control characters. Some browsers (like
# Chrome) ignore quite a few control characters at the start of a
# URL and might consider the URL as scheme relative.
if unicodedata.category(unicode(url[0]))[0] == 'C':
return False
return (not url_info[1] or url_info[1] == host) and \ return (not url_info[1] or url_info[1] == host) and \
(not url_info[0] or url_info[0] in ['http', 'https']) (not url_info[0] or url_info[0] in ['http', 'https'])

View File

@ -5,3 +5,22 @@ Django 1.4.20 release notes
*March 18, 2015* *March 18, 2015*
Django 1.4.20 fixes one security issue in 1.4.19. Django 1.4.20 fixes one security issue in 1.4.19.
Mitigated possible XSS attack via user-supplied redirect URLs
=============================================================
Django relies on user input in some cases (e.g.
:func:`django.contrib.auth.views.login` and :doc:`i18n </topics/i18n/index>`)
to redirect the user to an "on success" URL. The security checks for these
redirects (namely ``django.utils.http.is_safe_url()``) accepted URLs with
leading control characters and so considered URLs like ``\x08javascript:...``
safe. This issue doesn't affect Django currently, since we only put this URL
into the ``Location`` response header and browsers seem to ignore JavaScript
there. Browsers we tested also treat URLs prefixed with control characters such
as ``%08//example.com`` as relative paths so redirection to an unsafe target
isn't a problem either.
However, if a developer relies on ``is_safe_url()`` to
provide safe redirect targets and puts such a URL into a link, they could
suffer from an XSS attack as some browsers such as Google Chrome ignore control
characters at the start of a URL in an anchor ``href``.

View File

@ -98,7 +98,9 @@ class TestUtilsHttp(unittest.TestCase):
'http:\/example.com', 'http:\/example.com',
'http:/\example.com', 'http:/\example.com',
'javascript:alert("XSS")' 'javascript:alert("XSS")'
'\njavascript:alert(x)'): '\njavascript:alert(x)',
'\x08//example.com',
'\n'):
self.assertFalse(http.is_safe_url(bad_url, host='testserver'), "%s should be blocked" % bad_url) 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', for good_url in ('/view/?param=http://example.com',
'/view/?param=https://example.com', '/view/?param=https://example.com',