8000 [2.1.x] Fixed CVE-2018-14574 -- Fixed open redirect possibility in Co… · django/django@c4e5ff7 · GitHub
[go: up one dir, main page]
Skip to content

Commit c4e5ff7

Browse files
Andreas Hugtimgraham
Andreas Hug
authored and
timgraham
committed
[2.1.x] Fixed CVE-2018-14574 -- Fixed open redirect possibility in CommonMiddleware.
1 parent b323425 commit c4e5ff7

File tree

8 files changed

+78
-8
lines changed

8 files changed

+78
-8
lines changed

‎django/middleware/common.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from django.http import HttpResponsePermanentRedirect
88
from django.urls import is_valid_path
99
from django.utils.deprecation import MiddlewareMixin
10+
from django.utils.http import escape_leading_slashes
1011

1112

1213
class CommonMiddleware(MiddlewareMixin):
@@ -79,6 +80,8 @@ def get_full_path_with_slash(self, request):
7980
POST, PUT, or PATCH.
8081
"""
8182
new_path = request.get_full_path(force_append_slash=True)
83+
# Prevent construction of scheme relative urls.
84+
new_path = escape_leading_slashes(new_path)
8285
if settings.DEBUG and request.method in ('POST', 'PUT', 'PATCH'):
8386
raise RuntimeError(
8487
"You called this URL via %(method)s, but the URL doesn't end "

‎django/urls/resolvers.py‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from django.core.exceptions import ImproperlyConfigured
1818
from django.utils.datastructures import MultiValueDict
1919
from django.utils.functional import cached_property
20-
from django.utils.http import RFC3986_SUBDELIMS
20+
from django.utils.http import RFC3986_SUBDELIMS, escape_leading_slashes
2121
from django.utils.regex_helper import normalize
2222
from django.utils.translation import get_language
2323

@@ -592,9 +592,7 @@ def _reverse_with_prefix(self, lookup_view, _prefix, *args, **kwargs):
592592
# safe characters from `pchar` definition of RFC 3986
593593
url = quote(candidate_pat % text_candidate_subs, safe=RFC3986_SUBDELIMS + '/~:@')
594594
# Don't allow construction of scheme relative urls.
595-
if url.startswith('//'):
596-
url = '/%%2F%s' % url[2:]
597-
return url
595+
return escape_leading_slashes(url)
598596
# lookup_view can be URL name or callable, but callables are not
599597
# friendly in error messages.
600598
m = getattr(lookup_view, '__module__', None)

‎django/utils/http.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,3 +433,14 @@ def limited_parse_qsl(qs, keep_blank_values=False, encoding='utf-8',
433433
value = unquote(value, encoding=encoding, errors=errors)
434434
r.append((name, value))
435435
return r
436+
437+
438+
def escape_leading_slashes(url):
439+
"""
440+
If redirecting to an absolute path (two leading slashes), a slash must be
441+
escaped to prevent browsers from handling the path as schemaless and
442+
redirecting to another host.
443+
"""
444+
if url.startswith('//'):
445+
url = '/%2F{}'.format(url[2:])
446+
return url

‎docs/releases/1.11.15.txt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,16 @@ Django 1.11.15 release notes
55
*August 1, 2018*
66

77
Django 1.11.15 fixes a security issue in 1.11.14.
8+
9+
CVE-2018-14574: Open redirect possibility in ``CommonMiddleware``
10+
=================================================================
11+
12+
If the :class:`~django.middleware.common.CommonMiddleware` and the
13+
:setting:`APPEND_SLASH` setting are both enabled, and if the project has a
14+
URL pattern that accepts any path ending in a slash (many content management
15+
systems have such a pattern), then a request to a maliciously crafted URL of
16+
that site could lead to a redirect to another site, enabling phishing and other
17+
attacks.
18+
19+
``CommonMiddleware`` now escapes leading slashes to prevent redirects to other
20+
domains.

‎docs/releases/2.0.8.txt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,19 @@ Django 2.0.8 release notes
66

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

9+
CVE-2018-14574: Open redirect possibility in ``CommonMiddleware``
10+
=================================================================
11+
12+
If the :class:`~django.middleware.common.CommonMiddleware` and the
13+
:setting:`APPEND_SLASH` setting are both enabled, and if the project has a
14+
URL pattern that accepts any path ending in a slash (many content management
15+
systems have such a pattern), then a request to a maliciously crafted URL of
16+
that site could lead to a redirect to another site, enabling phishing and other
17+
attacks.
18+
19+
``CommonMiddleware`` now escapes leading slashes to prevent redirects to other
20+
domains.
21+
922
Bugfixes
1023
========
1124

‎tests/middleware/tests.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,25 @@ def test_append_slash_quoted(self):
130130
self.assertEqual(r.status_code, 301)
131131
self.assertEqual(r.url, '/needsquoting%23/')
132132

133+
@override_settings(APPEND_SLASH=True)
134+
def test_append_slash_leading_slashes(self):
135+
"""
136+
Paths starting with two slashes are escaped to prevent open redirects.
137+
If there's a URL pattern that allows paths to start with two slashes, a
138+
request with path //evil.com must not redirect to //evil.com/ (appended
139+
slash) which is a schemaless absolute URL. The browser would navigate
140+
to evil.com/.
141+
"""
142+
# Use 4 slashes because of RequestFactory behavior.
143+
request = self.rf.get('////evil.com/security')
144+
response = HttpResponseNotFound()
145+
r = CommonMiddleware().process_request(request)
146+
self.assertEqual(r.status_code, 301)
147+
self.assertEqual(r.url, '/%2Fevil.com/security/')
148+
r = CommonMiddleware().process_response(request, response)
149+
self.assertEqual(r.status_code, 301)
150+
self.assertEqual(r.url, '/%2Fevil.com/security/')
151+
133152
@override_settings(APPEND_SLASH=False, PREPEND_WWW=True)
134153
def test_prepend_www(self):
135154
request = self.rf.get('/path/')

‎tests/middleware/urls.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,6 @@
66
url(r'^noslash$', views.empty_view),
77
url(r'^slash/$', views.empty_view),
88
url(r'^needsquoting#/$', views.empty_view),
9+
# Accepts paths with two leading slashes.
10+
url(r'^(.+)/security/$', views.empty_view),
911
]

‎tests/utils_tests/test_http.py‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
from django.utils.datastructures import MultiValueDict
66
from django.utils.deprecation import RemovedInDjango30Warning
77
from django.utils.http import (
8-
base36_to_int, cookie_date, http_date, int_to_base36, is_safe_url,
9-
is_same_domain, parse_etags, parse_http_date, quote_etag, urlencode,
10-
urlquote, urlquote_plus, urlsafe_base64_decode, urlsafe_base64_encode,
11-
urlunquote, urlunquote_plus,
8+
base36_to_int, cookie_date, escape_leading_slashes, http_date,
9+
int_to_base36, is_safe_url, is_same_domain, parse_etags, parse_http_date,
10+
quote_etag, urlencode, urlquote, urlquote_plus, urlsafe_base64_decode,
11+
urlsafe_base64_encode, urlunquote, urlunquote_plus,
1212
)
1313

1414

@@ -271,3 +271,14 @@ def test_parsing_rfc850(self):
271271
def test_parsing_asctime(self):
272272
parsed = parse_http_date('Sun Nov 6 08:49:37 1994')
273273
self.assertEqual(datetime.utcfromtimestamp(parsed), datetime(1994, 11, 6, 8, 49, 37))
274+
275+
276+
class EscapeLeadingSlashesTests(unittest.TestCase):
277+
def test(self):
278+
tests = (
279+
('//example.com', '/%2Fexample.com'),
280+
('//', '/%2F'),
281+
)
282+
for url, expected in tests:
283+
with self.subTest(url=url):
284+
self.assertEqual(escape_leading_slashes(url), expected)

0 commit comments

Comments
 (0)
0