mirror of
				https://github.com/django/django.git
				synced 2025-10-31 09:41:08 +00:00 
			
		
		
		
	Fixed CVE-2018-14574 -- Fixed open redirect possibility in CommonMiddleware.
This commit is contained in:
		| @@ -7,6 +7,7 @@ from django.core.mail import mail_managers | ||||
| from django.http import HttpResponsePermanentRedirect | ||||
| from django.urls import is_valid_path | ||||
| from django.utils.deprecation import MiddlewareMixin | ||||
| from django.utils.http import escape_leading_slashes | ||||
|  | ||||
|  | ||||
| class CommonMiddleware(MiddlewareMixin): | ||||
| @@ -79,6 +80,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 " | ||||
|   | ||||
| @@ -17,7 +17,7 @@ from django.core.checks.urls import check_resolver | ||||
| 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 | ||||
|  | ||||
| @@ -592,9 +592,7 @@ class URLResolver: | ||||
|                     # 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) | ||||
|   | ||||
| @@ -435,3 +435,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 | ||||
|   | ||||
| @@ -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. | ||||
|   | ||||
| @@ -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 | ||||
| ======== | ||||
|  | ||||
|   | ||||
| @@ -130,6 +130,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/') | ||||
|   | ||||
| @@ -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), | ||||
| ] | ||||
|   | ||||
| @@ -5,10 +5,10 @@ from django.test import SimpleTestCase, ignore_warnings | ||||
| from django.utils.datastructures import MultiValueDict | ||||
| from django.utils.deprecation import RemovedInDjango30Warning | ||||
| 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, | ||||
| ) | ||||
|  | ||||
|  | ||||
| @@ -275,3 +275,14 @@ class HttpDateProcessingTests(unittest.TestCase): | ||||
|     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) | ||||
|   | ||||
		Reference in New Issue
	
	Block a user