Permalink
Browse files

[1.9.x] Fixed #27912, CVE-2017-7233 -- Fixed is_safe_url() with numer…

…ic URLs.

This is a security fix.
  • Loading branch information...
timgraham committed Mar 14, 2017
1 parent 5f1ffb0 commit 254326cb3682389f55f886804d2c43f7b9f23e4f
Showing with 93 additions and 3 deletions.
  1. +65 −2 django/utils/http.py
  2. +12 −0 docs/releases/1.8.18.txt
  3. +12 −0 docs/releases/1.9.13.txt
  4. +4 −1 tests/utils_tests/test_http.py
View
@@ -15,9 +15,20 @@
from django.utils.functional import allow_lazy
from django.utils.six.moves.urllib.parse import (
quote, quote_plus, unquote, unquote_plus, urlencode as original_urlencode,
urlparse,
)
if six.PY2:
from urlparse import (
ParseResult, SplitResult, _splitnetloc, _splitparams, scheme_chars,
uses_params,
)
_coerce_args = None
else:
from urllib.parse import (
ParseResult, SplitResult, _coerce_args, _splitnetloc, _splitparams,
scheme_chars, uses_params,
)
ETAG_MATCH = re.compile(r'(?:W/)?"((?:\\.|[^"])*)"')
MONTHS = 'jan feb mar apr may jun jul aug sep oct nov dec'.split()
@@ -300,12 +311,64 @@ def is_safe_url(url, host=None):
return _is_safe_url(url, host) and _is_safe_url(url.replace('\\', '/'), host)
# Copied from urllib.parse.urlparse() but uses fixed urlsplit() function.
def _urlparse(url, scheme='', allow_fragments=True):
"""Parse a URL into 6 components:
<scheme>://<netloc>/<path>;<params>?<query>#<fragment>
Return a 6-tuple: (scheme, netloc, path, params, query, fragment).
Note that we don't break the components up in smaller bits
(e.g. netloc is a single string) and we don't expand % escapes."""
if _coerce_args:
url, scheme, _coerce_result = _coerce_args(url, scheme)
splitresult = _urlsplit(url, scheme, allow_fragments)
scheme, netloc, url, query, fragment = splitresult
if scheme in uses_params and ';' in url:
url, params = _splitparams(url)
else:
params = ''
result = ParseResult(scheme, netloc, url, params, query, fragment)
return _coerce_result(result) if _coerce_args else result
# Copied from urllib.parse.urlsplit() with
# https://github.com/python/cpython/pull/661 applied.
def _urlsplit(url, scheme='', allow_fragments=True):
"""Parse a URL into 5 components:
<scheme>://<netloc>/<path>?<query>#<fragment>
Return a 5-tuple: (scheme, netloc, path, query, fragment).
Note that we don't break the components up in smaller bits
(e.g. netloc is a single string) and we don't expand % escapes."""
if _coerce_args:
url, scheme, _coerce_result = _coerce_args(url, scheme)
allow_fragments = bool(allow_fragments)
netloc = query = fragment = ''
i = url.find(':')
if i > 0:
for c in url[:i]:
if c not in scheme_chars:
break
else:
scheme, url = url[:i].lower(), url[i + 1:]
if url[:2] == '//':
netloc, url = _splitnetloc(url, 2)
if (('[' in netloc and ']' not in netloc) or
(']' in netloc and '[' not in netloc)):
raise ValueError("Invalid IPv6 URL")
if allow_fragments and '#' in url:
url, fragment = url.split('#', 1)
if '?' in url:
url, query = url.split('?', 1)
v = SplitResult(scheme, netloc, url, query, fragment)
return _coerce_result(v) if _coerce_args else v
def _is_safe_url(url, host):
# Chrome considers any URL with more than two slashes to be absolute, but
# urlparse is not so flexible. Treat any url with three slashes as unsafe.
if url.startswith('///'):
return False
url_info = urlparse(url)
url_info = _urlparse(url)
# Forbid URLs like http:///example.com - with a scheme, but without a hostname.
# In that URL, example.com is not the hostname but, a path component. However,
# Chrome will still consider example.com to be the hostname, so we must not
View
@@ -6,6 +6,18 @@ Django 1.8.18 release notes
Django 1.8.18 fixes two security issues in 1.8.17.
CVE-2017-7233: Open redirect and possible XSS attack via user-supplied numeric 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 check for these
redirects (namely ``django.utils.http.is_safe_url()``) considered some numeric
URLs (e.g. ``http:999999999``) "safe" when they shouldn't be.
Also, 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.
CVE-2017-7234: Open redirect vulnerability in ``django.views.static.serve()``
=============================================================================
View
@@ -7,6 +7,18 @@ Django 1.9.13 release notes
Django 1.9.13 fixes two security issues and a bug in 1.9.12. This is the final
release of the 1.9.x series.
CVE-2017-7233: Open redirect and possible XSS attack via user-supplied numeric 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 check for these
redirects (namely ``django.utils.http.is_safe_url()``) considered some numeric
URLs (e.g. ``http:999999999``) "safe" when they shouldn't be.
Also, 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.
CVE-2017-7234: Open redirect vulnerability in ``django.views.static.serve()``
=============================================================================
@@ -98,6 +98,8 @@ def test_is_safe_url(self):
r'http://testserver\me:pass@example.com',
r'http://testserver\@example.com',
r'http:\\testserver\confirm\me@example.com',
'http:999999999',
'ftp:9999999999',
'\n'):
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',
@@ -108,7 +110,8 @@ def test_is_safe_url(self):
'HTTPS://testserver/',
'//testserver/',
'http://testserver/confirm?email=me@example.com',
'/url%20with%20spaces/'):
'/url%20with%20spaces/',
'path/http:2222222222'):
self.assertTrue(http.is_safe_url(good_url, host='testserver'), "%s should be allowed" % good_url)
if six.PY2:

0 comments on commit 254326c

Please sign in to comment.