diff --git a/django/utils/http.py b/django/utils/http.py index 2d162a0fa55..f47a09a2dca 100644 --- a/django/utils/http.py +++ b/django/utils/http.py @@ -18,9 +18,20 @@ from django.utils.encoding import force_bytes, force_str, force_text from django.utils.functional import keep_lazy_text 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, + ) + # based on RFC 7232, Appendix C ETAG_MATCH = re.compile(r''' \A( # start of string and capture group @@ -319,12 +330,64 @@ def is_safe_url(url, host=None, allowed_hosts=None, require_https=False): _is_safe_url(url.replace('\\', '/'), allowed_hosts, require_https=require_https)) +# Copied from urllib.parse.urlparse() but uses fixed urlsplit() function. +def _urlparse(url, scheme='', allow_fragments=True): + """Parse a URL into 6 components: + :///;?# + 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: + :///?# + 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, allowed_hosts, require_https=False): # 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 diff --git a/docs/releases/1.10.7.txt b/docs/releases/1.10.7.txt index 0c79347cc3d..c5caa65143f 100644 --- a/docs/releases/1.10.7.txt +++ b/docs/releases/1.10.7.txt @@ -6,6 +6,18 @@ Django 1.10.7 release notes Django 1.10.7 fixes two security issues and a bug in 1.10.6. +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 `) +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()`` ============================================================================= diff --git a/docs/releases/1.8.18.txt b/docs/releases/1.8.18.txt index 7b1e08046cd..f41c7d080f9 100644 --- a/docs/releases/1.8.18.txt +++ b/docs/releases/1.8.18.txt @@ -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 `) +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()`` ============================================================================= diff --git a/docs/releases/1.9.13.txt b/docs/releases/1.9.13.txt index f9d203eafe8..4828096da9f 100644 --- a/docs/releases/1.9.13.txt +++ b/docs/releases/1.9.13.txt @@ -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 `) +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()`` ============================================================================= diff --git a/tests/utils_tests/test_http.py b/tests/utils_tests/test_http.py index e90d5e397d8..f6f711c72f4 100644 --- a/tests/utils_tests/test_http.py +++ b/tests/utils_tests/test_http.py @@ -106,6 +106,8 @@ class TestUtilsHttp(unittest.TestCase): r'http://testserver\me:pass@example.com', r'http://testserver\@example.com', r'http:\\testserver\confirm\me@example.com', + 'http:999999999', + 'ftp:9999999999', '\n', ) for bad_url in bad_urls: @@ -126,6 +128,7 @@ class TestUtilsHttp(unittest.TestCase): '//testserver/', 'http://testserver/confirm?email=me@example.com', '/url%20with%20spaces/', + 'path/http:2222222222', ) for good_url in good_urls: with ignore_warnings(category=RemovedInDjango21Warning):