Skip to content

Commit

Permalink
[2.0.x] Fixed CVE-2018-14574 -- Fixed open redirect possibility in Co…
Browse files Browse the repository at this point in the history
…mmonMiddleware.
  • Loading branch information
Andreas Hug authored and timgraham committed Jul 31, 2018
1 parent af34469 commit 6fffc3c
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 8 deletions.
3 changes: 3 additions & 0 deletions django/middleware/common.py
Expand Up @@ -11,6 +11,7 @@
cc_delim_re, get_conditional_response, set_response_etag,
)
from django.utils.deprecation import MiddlewareMixin, RemovedInDjango21Warning
from django.utils.http import escape_leading_slashes


class CommonMiddleware(MiddlewareMixin):
Expand Down Expand Up @@ -88,6 +89,8 @@ def get_full_path_with_slash(self, request):
POST, PUT, or PATCH.
"""
new_path = request.get_full_path(force_append_slash=True)
# Prevent construction of scheme relative urls.
new_path = escape_leading_slashes(new_path)
if settings.DEBUG and request.method in ('POST', 'PUT', 'PATCH'):
raise RuntimeError(
"You called this URL via %(method)s, but the URL doesn't end "
Expand Down
6 changes: 2 additions & 4 deletions django/urls/resolvers.py
Expand Up @@ -17,7 +17,7 @@
from django.core.exceptions import ImproperlyConfigured
from django.utils.datastructures import MultiValueDict
from django.utils.functional import cached_property
from django.utils.http import RFC3986_SUBDELIMS
from django.utils.http import RFC3986_SUBDELIMS, escape_leading_slashes
from django.utils.regex_helper import normalize
from django.utils.translation import get_language

Expand Down Expand Up @@ -604,9 +604,7 @@ def _reverse_with_prefix(self, lookup_view, _prefix, *args, **kwargs):
# safe characters from `pchar` definition of RFC 3986
url = quote(candidate_pat % text_candidate_subs, safe=RFC3986_SUBDELIMS + '/~:@')
# Don't allow construction of scheme relative urls.
if url.startswith('//'):
url = '/%%2F%s' % url[2:]
return url
return escape_leading_slashes(url)
# lookup_view can be URL name or callable, but callables are not
# friendly in error messages.
m = getattr(lookup_view, '__module__', None)
Expand Down
11 changes: 11 additions & 0 deletions django/utils/http.py
Expand Up @@ -437,3 +437,14 @@ def limited_parse_qsl(qs, keep_blank_values=False, encoding='utf-8',
value = unquote(value, encoding=encoding, errors=errors)
r.append((name, value))
return r


def escape_leading_slashes(url):
"""
If redirecting to an absolute path (two leading slashes), a slash must be
escaped to prevent browsers from handling the path as schemaless and
redirecting to another host.
"""
if url.startswith('//'):
url = '/%2F{}'.format(url[2:])
return url
13 changes: 13 additions & 0 deletions docs/releases/1.11.15.txt
Expand Up @@ -5,3 +5,16 @@ Django 1.11.15 release notes
*August 1, 2018*

Django 1.11.15 fixes a security issue in 1.11.14.

CVE-2018-14574: Open redirect possibility in ``CommonMiddleware``
=================================================================

If the :class:`~django.middleware.common.CommonMiddleware` and the
:setting:`APPEND_SLASH` setting are both enabled, and if the project has a
URL pattern that accepts any path ending in a slash (many content management
systems have such a pattern), then a request to a maliciously crafted URL of
that site could lead to a redirect to another site, enabling phishing and other
attacks.

``CommonMiddleware`` now escapes leading slashes to prevent redirects to other
domains.
13 changes: 13 additions & 0 deletions docs/releases/2.0.8.txt
Expand Up @@ -6,6 +6,19 @@ Django 2.0.8 release notes

Django 2.0.8 fixes a security issue and several bugs in 2.0.7.

CVE-2018-14574: Open redirect possibility in ``CommonMiddleware``
=================================================================

If the :class:`~django.middleware.common.CommonMiddleware` and the
:setting:`APPEND_SLASH` setting are both enabled, and if the project has a
URL pattern that accepts any path ending in a slash (many content management
systems have such a pattern), then a request to a maliciously crafted URL of
that site could lead to a redirect to another site, enabling phishing and other
attacks.

``CommonMiddleware`` now escapes leading slashes to prevent redirects to other
domains.

Bugfixes
========

Expand Down
19 changes: 19 additions & 0 deletions tests/middleware/tests.py
Expand Up @@ -133,6 +133,25 @@ def test_append_slash_quoted(self):
self.assertEqual(r.status_code, 301)
self.assertEqual(r.url, '/needsquoting%23/')

@override_settings(APPEND_SLASH=True)
def test_append_slash_leading_slashes(self):
"""
Paths starting with two slashes are escaped to prevent open redirects.
If there's a URL pattern that allows paths to start with two slashes, a
request with path //evil.com must not redirect to //evil.com/ (appended
slash) which is a schemaless absolute URL. The browser would navigate
to evil.com/.
"""
# Use 4 slashes because of RequestFactory behavior.
request = self.rf.get('////evil.com/security')
response = HttpResponseNotFound()
r = CommonMiddleware().process_request(request)
self.assertEqual(r.status_code, 301)
self.assertEqual(r.url, '/%2Fevil.com/security/')
r = CommonMiddleware().process_response(request, response)
self.assertEqual(r.status_code, 301)
self.assertEqual(r.url, '/%2Fevil.com/security/')

@override_settings(APPEND_SLASH=False, PREPEND_WWW=True)
def test_prepend_www(self):
request = self.rf.get('/path/')
Expand Down
2 changes: 2 additions & 0 deletions tests/middleware/urls.py
Expand Up @@ -6,4 +6,6 @@
url(r'^noslash$', views.empty_view),
url(r'^slash/$', views.empty_view),
url(r'^needsquoting#/$', views.empty_view),
# Accepts paths with two leading slashes.
url(r'^(.+)/security/$', views.empty_view),
]
19 changes: 15 additions & 4 deletions tests/utils_tests/test_http.py
Expand Up @@ -6,10 +6,10 @@
from django.utils.datastructures import MultiValueDict
from django.utils.deprecation import RemovedInDjango21Warning
from django.utils.http import (
base36_to_int, cookie_date, http_date, int_to_base36, is_safe_url,
is_same_domain, parse_etags, parse_http_date, quote_etag, urlencode,
urlquote, urlquote_plus, urlsafe_base64_decode, urlsafe_base64_encode,
urlunquote, urlunquote_plus,
base36_to_int, cookie_date, escape_leading_slashes, http_date,
int_to_base36, is_safe_url, is_same_domain, parse_etags, parse_http_date,
quote_etag, urlencode, urlquote, urlquote_plus, urlsafe_base64_decode,
urlsafe_base64_encode, urlunquote, urlunquote_plus,
)


Expand Down Expand Up @@ -275,3 +275,14 @@ def test_parsing_rfc850(self):
def test_parsing_asctime(self):
parsed = parse_http_date('Sun Nov 6 08:49:37 1994')
self.assertEqual(datetime.utcfromtimestamp(parsed), datetime(1994, 11, 6, 8, 49, 37))


class EscapeLeadingSlashesTests(unittest.TestCase):
def test(self):
tests = (
('//example.com', '/%2Fexample.com'),
('//', '/%2F'),
)
for url, expected in tests:
with self.subTest(url=url):
self.assertEqual(escape_leading_slashes(url), expected)

0 comments on commit 6fffc3c

Please sign in to comment.