10000 [1.8.x] Made is_safe_url() reject URLs that start with control charac… · ddriddle/django@770427c · GitHub
[go: up one dir, main page]
Skip to content

Commit 770427c

Browse files
timgrahamtimgraham
committed
[1.8.x] Made is_safe_url() reject URLs that start with control characters.
This is a security fix; disclosure to follow shortly.
1 parent 5447709 commit 770427c

File tree

5 files changed

+68
-2
lines changed

5 files changed

+68
-2
lines changed

‎django/utils/http.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import datetime
66
import re
77
import sys
8+
import unicodedata
89
from binascii import Error as BinasciiError
910
from email.utils import formatdate
1011

@@ -272,9 +273,10 @@ def is_safe_url(url, host=None):
272273
273274
Always returns ``False`` on an empty url.
274275
"""
276+
if url is not None:
277+
url = url.strip()
275278
if not url:
276279
return False
277-
url = url.strip()
278280
# Chrome treats \ completely as /
279281
url = url.replace('\\', '/')
280282
# Chrome considers any URL with more than two slashes to be absolute, but
@@ -288,5 +290,10 @@ def is_safe_url(url, host=None):
288290
# allow this syntax.
289291
if not url_info.netloc and url_info.scheme:
290292
return False
293+
# Forbid URLs that start with control characters. Some browsers (like
294+
# Chrome) ignore quite a few control characters at the start of a
295+
# URL and might consider the URL as scheme relative.
296+
if unicodedata.category(url[0])[0] == 'C':
297+
return False
291298
return ((not url_info.netloc or url_info.netloc == host) and
292299
(not url_info.scheme or url_info.scheme in ['http', 'https']))

‎docs/releases/1.4.20.txt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,22 @@ Django 1.4.20 release notes
55
*March 18, 2015*
66

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

‎docs/releases/1.6.11.txt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,22 @@ it detects the length of the string it's processing increases. Remember that
2222
absolutely NO guarantee is provided about the results of ``strip_tags()`` being
2323
HTML safe. So NEVER mark safe the result of a ``strip_tags()`` call without
2424
escaping it first, for example with :func:`~django.utils.html.escape`.
25+
26+
Mitigated possible XSS attack via user-supplied redirect URLs
27+
=============================================================
28+
29+
Django relies on user input in some cases (e.g.
30+
:func:`django.contrib.auth.views.login` and :doc:`i18n </topics/i18n/index>`)
31+
to redirect the user to an "on success" URL. The security checks for these
32+
redirects (namely ``django.utils.http.is_safe_url()``) accepted URLs with
33+
leading control characters and so considered URLs like ``\x08javascript:...``
34+
safe. This issue doesn't affect Django currently, since we only put this URL
35+
into the ``Location`` response header and browsers seem to ignore JavaScript
36+
there. Browsers we tested also treat URLs prefixed with control characters such
37+
as ``%08//example.com`` as relative paths so redirection to an unsafe target
38+
isn't a problem either.
39+
40+
However, if a developer relies on ``is_safe_url()`` to
41+
provide safe redirect targets and puts such a URL into a link, they could
42+
suffer from an XSS attack as some browsers such as Google Chrome ignore control
43+
characters at the start of a URL in an anchor ``href``.

‎docs/releases/1.7.7.txt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,25 @@ absolutely NO guarantee is provided about the results of ``strip_tags()`` being
2323
HTML safe. So NEVER mark safe the result of a ``strip_tags()`` call without
2424
escaping it first, for example with :func:`~django.utils.html.escape`.
2525

26+
Mitigated possible XSS attack via user-supplied redirect URLs
27+
=============================================================
28+
29+
Django relies on user input in some cases (e.g.
30+
:func:`django.contrib.auth.views.login` and :doc:`i18n </topics/i18n/index>`)
31+
to redirect the user to an "on success" URL. The security checks for these
32+
redirects (namely ``django.utils.http.is_safe_url()``) accepted URLs with
33+
leading control characters and so considered URLs like ``\x08javascript:...``
34+
safe. This issue doesn't affect Django currently, since we only put this URL
35+
into the ``Location`` response header and browsers seem to ignore JavaScript
36+
there. Browsers we tested also treat URLs prefixed with control characters such
37+
as ``%08//example.com`` as relative paths so redirection to an unsafe target
38+
isn't a problem either.
39+
40+
However, if a developer relies on ``is_safe_url()`` to
41+
provide safe redirect targets and puts such a URL into a link, they could
42+
suffer from an XSS attack as some browsers such as Google Chrome ignore control
43+
characters at the start of a URL in an anchor ``href``.
44+
2645
Bugfixes
2746
========
2847

‎tests/utils_tests/test_http.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ def test_is_safe_url(self):
115115
'http:\/example.com',
116116
'http:/\example.com',
117117
'javascript:alert("XSS")',
118-
'\njavascript:alert(x)'):
118+
'\njavascript:alert(x)',
119+
'\x08//example.com',
120+
'\n'):
119121
self.assertFalse(http.is_safe_url(bad_url, host='testserver'), "%s should be blocked" % bad_url)
120122
for good_url in ('/view/?param=http://example.com',
121123
'/view/?param=https://example.com',

0 commit comments

Comments
 (0)
0