From 69b5e667385db9ed5e61917a58a75f97b6a97e68 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Wed, 3 Dec 2014 16:14:00 -0500 Subject: [PATCH] Fixed is_safe_url() to handle leading whitespace. This is a security fix. Disclosure following shortly. --- django/utils/http.py | 1 + docs/releases/1.4.18.txt | 14 ++++++++++++++ docs/releases/1.6.10.txt | 14 ++++++++++++++ docs/releases/1.7.3.txt | 14 ++++++++++++++ tests/utils_tests/test_http.py | 3 ++- 5 files changed, 45 insertions(+), 1 deletion(-) diff --git a/django/utils/http.py b/django/utils/http.py index c8411e5207..431f83df21 100644 --- a/django/utils/http.py +++ b/django/utils/http.py @@ -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 diff --git a/docs/releases/1.4.18.txt b/docs/releases/1.4.18.txt index 55256cfdf3..2da42533bd 100644 --- a/docs/releases/1.4.18.txt +++ b/docs/releases/1.4.18.txt @@ -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 `) +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 ======== diff --git a/docs/releases/1.6.10.txt b/docs/releases/1.6.10.txt index dafee70c8c..92b709d25d 100644 --- a/docs/releases/1.6.10.txt +++ b/docs/releases/1.6.10.txt @@ -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 `) +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. diff --git a/docs/releases/1.7.3.txt b/docs/releases/1.7.3.txt index 20d0b59457..0980c67d9d 100644 --- a/docs/releases/1.7.3.txt +++ b/docs/releases/1.7.3.txt @@ -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 `) +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 ======== diff --git a/tests/utils_tests/test_http.py b/tests/utils_tests/test_http.py index b3e2c7b8d3..ac103f15d6 100644 --- a/tests/utils_tests/test_http.py +++ b/tests/utils_tests/test_http.py @@ -109,7 +109,8 @@ class TestUtilsHttp(unittest.TestCase): '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',