summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndreas Hug <andreas.hug@moccu.com>2018-07-24 16:18:17 -0400
committerTim Graham <timograham@gmail.com>2018-07-25 12:13:03 -0400
commitd6eaee092709aad477a9894598496c6deec532ff (patch)
tree1b0a5d386ed8b6915cd3507bc2665ded4f3beb3d
parent4fd1f6702aaa9ca9b9918dbdb79546557a00d331 (diff)
downloaddjango-d6eaee092709aad477a9894598496c6deec532ff.tar.gz
[1.11.x] Fixed CVE-2018-14574 -- Fixed open redirect possibility in CommonMiddleware.
-rw-r--r--django/middleware/common.py3
-rw-r--r--django/urls/resolvers.py8
-rw-r--r--django/utils/http.py11
-rw-r--r--docs/releases/1.11.15.txt13
-rw-r--r--tests/middleware/tests.py19
-rw-r--r--tests/middleware/urls.py2
-rw-r--r--tests/utils_tests/test_http.py10
7 files changed, 62 insertions, 4 deletions
diff --git a/django/middleware/common.py b/django/middleware/common.py
index d18d23fa43..fff46ba552 100644
--- a/django/middleware/common.py
+++ b/django/middleware/common.py
@@ -11,6 +11,7 @@ from django.utils.cache import (
)
from django.utils.deprecation import MiddlewareMixin, RemovedInDjango21Warning
from django.utils.encoding import force_text
+from django.utils.http import escape_leading_slashes
from django.utils.six.moves.urllib.parse import urlparse
@@ -90,6 +91,8 @@ class CommonMiddleware(MiddlewareMixin):
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 "
diff --git a/django/urls/resolvers.py b/django/urls/resolvers.py
index 1de59a8763..25e9ae8276 100644
--- a/django/urls/resolvers.py
+++ b/django/urls/resolvers.py
@@ -20,7 +20,9 @@ from django.utils import lru_cache, six
from django.utils.datastructures import MultiValueDict
from django.utils.encoding import force_str, force_text
from django.utils.functional import cached_property
-from django.utils.http import RFC3986_SUBDELIMS, urlquote
+from django.utils.http import (
+ RFC3986_SUBDELIMS, escape_leading_slashes, urlquote,
+)
from django.utils.regex_helper import normalize
from django.utils.translation import get_language
@@ -465,9 +467,7 @@ class RegexURLResolver(LocaleRegexProvider):
# safe characters from `pchar` definition of RFC 3986
url = urlquote(candidate_pat % candidate_subs, safe=RFC3986_SUBDELIMS + str('/~:@'))
# 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)
diff --git a/django/utils/http.py b/django/utils/http.py
index 1fbc11b6fb..644d4d09fd 100644
--- a/django/utils/http.py
+++ b/django/utils/http.py
@@ -466,3 +466,14 @@ def limited_parse_qsl(qs, keep_blank_values=False, encoding='utf-8',
value = unquote(nv[1].replace(b'+', b' '))
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
diff --git a/docs/releases/1.11.15.txt b/docs/releases/1.11.15.txt
index 397681d52e..fca551e772 100644
--- a/docs/releases/1.11.15.txt
+++ b/docs/releases/1.11.15.txt
@@ -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.
diff --git a/tests/middleware/tests.py b/tests/middleware/tests.py
index 1d473ef558..d9d701f22a 100644
--- a/tests/middleware/tests.py
+++ b/tests/middleware/tests.py
@@ -137,6 +137,25 @@ class CommonMiddlewareTest(SimpleTestCase):
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/')
diff --git a/tests/middleware/urls.py b/tests/middleware/urls.py
index 8c6621d059..d623e7d6af 100644
--- a/tests/middleware/urls.py
+++ b/tests/middleware/urls.py
@@ -6,4 +6,6 @@ urlpatterns = [
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),
]
diff --git a/tests/utils_tests/test_http.py b/tests/utils_tests/test_http.py
index b435e33e44..d339e8a79c 100644
--- a/tests/utils_tests/test_http.py
+++ b/tests/utils_tests/test_http.py
@@ -248,3 +248,13 @@ class HttpDateProcessingTests(unittest.TestCase):
def test_parsing_asctime(self):
parsed = http.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:
+ self.assertEqual(http.escape_leading_slashes(url), expected)