Skip to content

Commit

Permalink
[1.4.x] Made is_safe_url() reject URLs that start with control charac…
Browse files Browse the repository at this point in the history
…ters.

This is a security fix; disclosure to follow shortly.
  • Loading branch information
timgraham committed Mar 18, 2015
1 parent 3b20558 commit 2342693
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 2 deletions.
9 changes: 8 additions & 1 deletion django/utils/http.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
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
Expand Down Expand Up @@ -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
Expand All @@ -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'])
19 changes: 19 additions & 0 deletions docs/releases/1.4.20.txt
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -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``.
4 changes: 3 additions & 1 deletion tests/regressiontests/utils/http.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ def test_is_safe_url(self):
'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',
Expand Down

0 comments on commit 2342693

Please sign in to comment.