Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fixed is_safe_url() to handle leading whitespace.
This is a security fix. Disclosure following shortly.
  • Loading branch information
timgraham committed Jan 13, 2015
1 parent 316b8d4 commit 69b5e66
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 1 deletion.
1 change: 1 addition & 0 deletions django/utils/http.py
Expand Up @@ -267,6 +267,7 @@ def is_safe_url(url, host=None):
"""
if not url:
return False
url = url.strip()
# Chrome treats \ completely as /
url = url.replace('\\', '/')
# Chrome considers any URL with more than two slashes to be absolute, but
Expand Down
14 changes: 14 additions & 0 deletions docs/releases/1.4.18.txt
Expand Up @@ -31,6 +31,20 @@ development server now does the same. Django's development server is not
recommended for production use, but matching the behavior of common production
servers reduces the surface area for behavior changes during deployment.

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.util.http.is_safe_url()``) didn't strip leading
whitespace on the tested URL and as such considered URLs like
``\njavascript:...`` safe. If a developer relied on ``is_safe_url()`` to
provide safe redirect targets and put such a URL into a link, they could suffer
from a XSS attack. This bug doesn't affect Django currently, since we only put
this URL into the ``Location`` response header and browsers seem to ignore
JavaScript there.

Bugfixes
========

Expand Down
14 changes: 14 additions & 0 deletions docs/releases/1.6.10.txt
Expand Up @@ -29,3 +29,17 @@ containing underscores from incoming requests by default. Django's built-in
development server now does the same. Django's development server is not
recommended for production use, but matching the behavior of common production
servers reduces the surface area for behavior changes during deployment.

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.util.http.is_safe_url()``) didn't strip leading
whitespace on the tested URL and as such considered URLs like
``\njavascript:...`` safe. If a developer relied on ``is_safe_url()`` to
provide safe redirect targets and put such a URL into a link, they could suffer
from a XSS attack. This bug doesn't affect Django currently, since we only put
this URL into the ``Location`` response header and browsers seem to ignore
JavaScript there.
14 changes: 14 additions & 0 deletions docs/releases/1.7.3.txt
Expand Up @@ -30,6 +30,20 @@ development server now does the same. Django's development server is not
recommended for production use, but matching the behavior of common production
servers reduces the surface area for behavior changes during deployment.

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.util.http.is_safe_url()``) didn't strip leading
whitespace on the tested URL and as such considered URLs like
``\njavascript:...`` safe. If a developer relied on ``is_safe_url()`` to
provide safe redirect targets and put such a URL into a link, they could suffer
from a XSS attack. This bug doesn't affect Django currently, since we only put
this URL into the ``Location`` response header and browsers seem to ignore
JavaScript there.

Bugfixes
========

Expand Down
3 changes: 2 additions & 1 deletion tests/utils_tests/test_http.py
Expand Up @@ -109,7 +109,8 @@ def test_is_safe_url(self):
'http:/\//example.com',
'http:\/example.com',
'http:/\example.com',
'javascript:alert("XSS")'):
'javascript:alert("XSS")',
'\njavascript:alert(x)'):
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',
Expand Down

0 comments on commit 69b5e66

Please sign in to comment.