Fixed #28488 -- Reallowed error handlers to access CSRF tokens.

Regression in eef95ea96f.
This commit is contained in:
Florian Apolloner 2017-09-17 22:24:05 +02:00 committed by Tim Graham
parent 77f82c4bf1
commit c4c128d67c
5 changed files with 57 additions and 6 deletions

View File

@ -194,15 +194,16 @@ class CsrfViewMiddleware(MiddlewareMixin):
# Set the Vary header since content varies with the CSRF cookie. # Set the Vary header since content varies with the CSRF cookie.
patch_vary_headers(response, ('Cookie',)) patch_vary_headers(response, ('Cookie',))
def process_view(self, request, callback, callback_args, callback_kwargs): def process_request(self, request):
if getattr(request, 'csrf_processing_done', False):
return None
csrf_token = self._get_token(request) csrf_token = self._get_token(request)
if csrf_token is not None: if csrf_token is not None:
# Use same token next time. # Use same token next time.
request.META['CSRF_COOKIE'] = csrf_token request.META['CSRF_COOKIE'] = csrf_token
def process_view(self, request, callback, callback_args, callback_kwargs):
if getattr(request, 'csrf_processing_done', False):
return None
# Wait until request.META["CSRF_COOKIE"] has been manipulated before # Wait until request.META["CSRF_COOKIE"] has been manipulated before
# bailing out, so that get_token still works # bailing out, so that get_token still works
if getattr(callback, 'csrf_exempt', False): if getattr(callback, 'csrf_exempt', False):
@ -274,6 +275,7 @@ class CsrfViewMiddleware(MiddlewareMixin):
reason = REASON_BAD_REFERER % referer.geturl() reason = REASON_BAD_REFERER % referer.geturl()
return self._reject(request, reason) return self._reject(request, reason)
csrf_token = request.META.get('CSRF_COOKIE')
if csrf_token is None: if csrf_token is None:
# No CSRF cookie. For POST requests, we insist on a CSRF cookie, # No CSRF cookie. For POST requests, we insist on a CSRF cookie,
# and in this way we can avoid all CSRF attacks, including login # and in this way we can avoid all CSRF attacks, including login

View File

@ -14,3 +14,7 @@ Bugfixes
* Fixed crash when using the name of a model's autogenerated primary key * Fixed crash when using the name of a model's autogenerated primary key
(``id``) in an ``Index``'s ``fields`` (:ticket:`28597`). (``id``) in an ``Index``'s ``fields`` (:ticket:`28597`).
* Fixed a regression in Django 1.9 where a custom view error handler such as
``handler404`` that accesses ``csrf_token`` could cause CSRF verification
failures on other pages (:ticket:`28488`).

View File

@ -0,0 +1,3 @@
urlpatterns = []
handler404 = 'csrf_tests.views.csrf_token_error_handler'

View File

@ -84,6 +84,7 @@ class CsrfViewMiddlewareTestMixin:
# does use the csrf request processor. By using this, we are testing # does use the csrf request processor. By using this, we are testing
# that the view processor is properly lazy and doesn't call get_token() # that the view processor is properly lazy and doesn't call get_token()
# until needed. # until needed.
self.mw.process_request(req)
self.mw.process_view(req, non_token_view_using_request_processor, (), {}) self.mw.process_view(req, non_token_view_using_request_processor, (), {})
resp = non_token_view_using_request_processor(req) resp = non_token_view_using_request_processor(req)
resp2 = self.mw.process_response(req, resp) resp2 = self.mw.process_response(req, resp)
@ -99,6 +100,7 @@ class CsrfViewMiddlewareTestMixin:
""" """
with patch_logger('django.security.csrf', 'warning') as logger_calls: with patch_logger('django.security.csrf', 'warning') as logger_calls:
req = self._get_POST_no_csrf_cookie_request() req = self._get_POST_no_csrf_cookie_request()
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertEqual(403, req2.status_code) self.assertEqual(403, req2.status_code)
self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_NO_CSRF_COOKIE) self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_NO_CSRF_COOKIE)
@ -110,6 +112,7 @@ class CsrfViewMiddlewareTestMixin:
""" """
with patch_logger('django.security.csrf', 'warning') as logger_calls: with patch_logger('django.security.csrf', 'warning') as logger_calls:
req = self._get_POST_csrf_cookie_request() req = self._get_POST_csrf_cookie_request()
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertEqual(403, req2.status_code) self.assertEqual(403, req2.status_code)
self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_BAD_TOKEN) self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_BAD_TOKEN)
@ -119,6 +122,7 @@ class CsrfViewMiddlewareTestMixin:
If both a cookie and a token is present, the middleware lets it through. If both a cookie and a token is present, the middleware lets it through.
""" """
req = self._get_POST_request_with_token() req = self._get_POST_request_with_token()
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
@ -128,6 +132,7 @@ class CsrfViewMiddlewareTestMixin:
has been applied to the view, the middleware lets it through has been applied to the view, the middleware lets it through
""" """
req = self._get_POST_csrf_cookie_request() req = self._get_POST_csrf_cookie_request()
self.mw.process_request(req)
req2 = self.mw.process_view(req, csrf_exempt(post_form_view), (), {}) req2 = self.mw.process_view(req, csrf_exempt(post_form_view), (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
@ -137,6 +142,7 @@ class CsrfViewMiddlewareTestMixin:
""" """
req = self._get_POST_csrf_cookie_request() req = self._get_POST_csrf_cookie_request()
req.META['HTTP_X_CSRFTOKEN'] = self._csrf_id req.META['HTTP_X_CSRFTOKEN'] = self._csrf_id
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
@ -147,6 +153,7 @@ class CsrfViewMiddlewareTestMixin:
""" """
req = self._get_POST_csrf_cookie_request() req = self._get_POST_csrf_cookie_request()
req.META['HTTP_X_CSRFTOKEN_CUSTOMIZED'] = self._csrf_id req.META['HTTP_X_CSRFTOKEN_CUSTOMIZED'] = self._csrf_id
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
@ -175,12 +182,14 @@ class CsrfViewMiddlewareTestMixin:
req = self._get_GET_csrf_cookie_request() req = self._get_GET_csrf_cookie_request()
req.method = 'PUT' req.method = 'PUT'
req.META['HTTP_X_CSRFTOKEN'] = self._csrf_id req.META['HTTP_X_CSRFTOKEN'] = self._csrf_id
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
req = self._get_GET_csrf_cookie_request() req = self._get_GET_csrf_cookie_request()
req.method = 'DELETE' req.method = 'DELETE'
req.META['HTTP_X_CSRFTOKEN'] = self._csrf_id req.META['HTTP_X_CSRFTOKEN'] = self._csrf_id
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
@ -214,6 +223,7 @@ class CsrfViewMiddlewareTestMixin:
CsrfTokenNode works when a CSRF cookie is set. CsrfTokenNode works when a CSRF cookie is set.
""" """
req = self._get_GET_csrf_cookie_request() req = self._get_GET_csrf_cookie_request()
self.mw.process_request(req)
self.mw.process_view(req, token_view, (), {}) self.mw.process_view(req, token_view, (), {})
resp = token_view(req) resp = token_view(req)
self._check_token_present(resp) self._check_token_present(resp)
@ -223,6 +233,7 @@ class CsrfViewMiddlewareTestMixin:
get_token still works for a view decorated with 'csrf_exempt'. get_token still works for a view decorated with 'csrf_exempt'.
""" """
req = self._get_GET_csrf_cookie_request() req = self._get_GET_csrf_cookie_request()
self.mw.process_request(req)
self.mw.process_view(req, csrf_exempt(token_view), (), {}) self.mw.process_view(req, csrf_exempt(token_view), (), {})
resp = token_view(req) resp = token_view(req)
self._check_token_present(resp) self._check_token_present(resp)
@ -254,6 +265,7 @@ class CsrfViewMiddlewareTestMixin:
requests. If it appears in the response, it should keep its value. requests. If it appears in the response, it should keep its value.
""" """
req = self._get_POST_request_with_token() req = self._get_POST_request_with_token()
self.mw.process_request(req)
self.mw.process_view(req, token_view, (), {}) self.mw.process_view(req, token_view, (), {})
resp = token_view(req) resp = token_view(req)
resp = self.mw.process_response(req, resp) resp = self.mw.process_response(req, resp)
@ -327,6 +339,7 @@ class CsrfViewMiddlewareTestMixin:
req._is_secure_override = True req._is_secure_override = True
req.META['HTTP_HOST'] = 'www.example.com' req.META['HTTP_HOST'] = 'www.example.com'
req.META['HTTP_REFERER'] = 'https://www.example.com/somepage' req.META['HTTP_REFERER'] = 'https://www.example.com/somepage'
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
@ -341,6 +354,7 @@ class CsrfViewMiddlewareTestMixin:
req._is_secure_override = True req._is_secure_override = True
req.META['HTTP_HOST'] = 'www.example.com' req.META['HTTP_HOST'] = 'www.example.com'
req.META['HTTP_REFERER'] = 'https://www.example.com' req.META['HTTP_REFERER'] = 'https://www.example.com'
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
@ -354,6 +368,7 @@ class CsrfViewMiddlewareTestMixin:
'HTTP_X_FORWARDED_HOST': 'www.example.com', 'HTTP_X_FORWARDED_HOST': 'www.example.com',
'HTTP_X_FORWARDED_PORT': '443', 'HTTP_X_FORWARDED_PORT': '443',
}) })
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
@ -367,6 +382,7 @@ class CsrfViewMiddlewareTestMixin:
req._is_secure_override = True req._is_secure_override = True
req.META['HTTP_HOST'] = 'www.example.com' req.META['HTTP_HOST'] = 'www.example.com'
req.META['HTTP_REFERER'] = 'https://dashboard.example.com' req.META['HTTP_REFERER'] = 'https://dashboard.example.com'
self.mw.process_request(req)
req2 = self.mw.process_view(req, post_form_view, (), {}) req2 = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
@ -380,6 +396,7 @@ class CsrfViewMiddlewareTestMixin:
req._is_secure_override = True req._is_secure_override = True
req.META['HTTP_HOST'] = 'www.example.com' req.META['HTTP_HOST'] = 'www.example.com'
req.META['HTTP_REFERER'] = 'https://dashboard.example.com' req.META['HTTP_REFERER'] = 'https://dashboard.example.com'
self.mw.process_request(req)
response = self.mw.process_view(req, post_form_view, (), {}) response = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(response) self.assertIsNone(response)
@ -388,6 +405,7 @@ class CsrfViewMiddlewareTestMixin:
req._is_secure_override = True req._is_secure_override = True
req.META['HTTP_REFERER'] = 'https://foo.example.com/' req.META['HTTP_REFERER'] = 'https://foo.example.com/'
req.META['SERVER_PORT'] = '443' req.META['SERVER_PORT'] = '443'
self.mw.process_request(req)
response = self.mw.process_view(req, post_form_view, (), {}) response = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(response) self.assertIsNone(response)
@ -397,6 +415,7 @@ class CsrfViewMiddlewareTestMixin:
req.META['HTTP_HOST'] = 'www.example.com' req.META['HTTP_HOST'] = 'www.example.com'
req.META['HTTP_REFERER'] = 'https://foo.example.com:4443/' req.META['HTTP_REFERER'] = 'https://foo.example.com:4443/'
req.META['SERVER_PORT'] = '4443' req.META['SERVER_PORT'] = '4443'
self.mw.process_request(req)
response = self.mw.process_view(req, post_form_view, (), {}) response = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(response) self.assertIsNone(response)
@ -461,11 +480,13 @@ class CsrfViewMiddlewareTestMixin:
token = ('ABC' + self._csrf_id)[:CSRF_TOKEN_LENGTH] token = ('ABC' + self._csrf_id)[:CSRF_TOKEN_LENGTH]
req = CsrfPostRequest(token, raise_error=False) req = CsrfPostRequest(token, raise_error=False)
self.mw.process_request(req)
resp = self.mw.process_view(req, post_form_view, (), {}) resp = self.mw.process_view(req, post_form_view, (), {})
self.assertIsNone(resp) self.assertIsNone(resp)
req = CsrfPostRequest(token, raise_error=True) req = CsrfPostRequest(token, raise_error=True)
with patch_logger('django.security.csrf', 'warning') as logger_calls: with patch_logger('django.security.csrf', 'warning') as logger_calls:
self.mw.process_request(req)
resp = self.mw.process_view(req, post_form_view, (), {}) resp = self.mw.process_view(req, post_form_view, (), {})
self.assertEqual(resp.status_code, 403) self.assertEqual(resp.status_code, 403)
self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_BAD_TOKEN) self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_BAD_TOKEN)
@ -585,6 +606,7 @@ class CsrfViewMiddlewareTests(CsrfViewMiddlewareTestMixin, SimpleTestCase):
The csrf token is reset from a bare secret. The csrf token is reset from a bare secret.
""" """
req = self._get_POST_bare_secret_csrf_cookie_request_with_token() req = self._get_POST_bare_secret_csrf_cookie_request_with_token()
self.mw.process_request(req)
req2 = self.mw.process_view(req, token_view, (), {}) req2 = self.mw.process_view(req, token_view, (), {})
self.assertIsNone(req2) self.assertIsNone(req2)
resp = token_view(req) resp = token_view(req)
@ -656,7 +678,7 @@ class CsrfViewMiddlewareUseSessionsTests(CsrfViewMiddlewareTestMixin, SimpleTest
'SessionMiddleware must appear before CsrfViewMiddleware in MIDDLEWARE.' 'SessionMiddleware must appear before CsrfViewMiddleware in MIDDLEWARE.'
) )
with self.assertRaisesMessage(ImproperlyConfigured, msg): with self.assertRaisesMessage(ImproperlyConfigured, msg):
self.mw.process_view(HttpRequest(), None, (), {}) self.mw.process_request(HttpRequest())
def test_process_response_get_token_used(self): def test_process_response_get_token_used(self):
"""The ensure_csrf_cookie() decorator works without middleware.""" """The ensure_csrf_cookie() decorator works without middleware."""
@ -730,3 +752,16 @@ class CsrfViewMiddlewareUseSessionsTests(CsrfViewMiddlewareTestMixin, SimpleTest
'Referer checking failed - Referer is insecure while host is secure.', 'Referer checking failed - Referer is insecure while host is secure.',
status_code=403, status_code=403,
) )
@override_settings(ROOT_URLCONF='csrf_tests.csrf_token_error_handler_urls', DEBUG=False)
class CsrfInErrorHandlingViewsTests(SimpleTestCase):
def test_csrf_token_on_404_stays_constant(self):
response = self.client.get('/does not exist/')
# The error handler returns status code 599.
self.assertEqual(response.status_code, 599)
token1 = response.content
response = self.client.get('/does not exist/')
self.assertEqual(response.status_code, 599)
token2 = response.content
self.assertTrue(equivalent_tokens(token1.decode('ascii'), token2.decode('ascii')))

View File

@ -1,5 +1,6 @@
from django.http import HttpResponse from django.http import HttpResponse
from django.template import RequestContext, Template from django.middleware.csrf import get_token
from django.template import Context, RequestContext, Template
from django.template.context_processors import csrf from django.template.context_processors import csrf
from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.csrf import ensure_csrf_cookie
@ -28,3 +29,9 @@ def non_token_view_using_request_processor(request):
context = RequestContext(request, processors=[csrf]) context = RequestContext(request, processors=[csrf])
template = Template('') template = Template('')
return HttpResponse(template.render(context)) return HttpResponse(template.render(context))
def csrf_token_error_handler(request, **kwargs):
"""This error handler accesses the CSRF token."""
template = Template(get_token(request))
return HttpResponse(template.render(Context()), status=599)