[1.8.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 5447709a57
commit 770427c289
5 changed files with 68 additions and 2 deletions

View File

@ -5,6 +5,7 @@ import calendar
import datetime import datetime
import re import re
import sys import sys
import unicodedata
from binascii import Error as BinasciiError from binascii import Error as BinasciiError
from email.utils import formatdate from email.utils import formatdate
@ -272,9 +273,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
@ -288,5 +290,10 @@ def is_safe_url(url, host=None):
# allow this syntax. # allow this syntax.
if not url_info.netloc and url_info.scheme: if not url_info.netloc and url_info.scheme:
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(url[0])[0] == 'C':
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']))

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

@ -22,3 +22,22 @@ it detects the length of the string it's processing increases. Remember that
absolutely NO guarantee is provided about the results of ``strip_tags()`` being absolutely NO guarantee is provided about the results of ``strip_tags()`` being
HTML safe. So NEVER mark safe the result of a ``strip_tags()`` call without HTML safe. So NEVER mark safe the result of a ``strip_tags()`` call without
escaping it first, for example with :func:`~django.utils.html.escape`. escaping it first, for example with :func:`~django.utils.html.escape`.
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

@ -23,6 +23,25 @@ absolutely NO guarantee is provided about the results of ``strip_tags()`` being
HTML safe. So NEVER mark safe the result of a ``strip_tags()`` call without HTML safe. So NEVER mark safe the result of a ``strip_tags()`` call without
escaping it first, for example with :func:`~django.utils.html.escape`. escaping it first, for example with :func:`~django.utils.html.escape`.
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``.
Bugfixes Bugfixes
======== ========

View File

@ -115,7 +115,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',