From 55775891fbfd8679b75336aa2f15ff9190e9f7a8 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Sat, 29 May 2021 04:49:53 -0700 Subject: [PATCH] Fixed #32795 -- Changed CsrfViewMiddleware to fail earlier on badly formatted tokens. --- django/middleware/csrf.py | 42 +++++++++++++++++++++++++++++---------- tests/csrf_tests/tests.py | 19 ++++++++++-------- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/django/middleware/csrf.py b/django/middleware/csrf.py index 9e6c9f5e9d..3febfd9486 100644 --- a/django/middleware/csrf.py +++ b/django/middleware/csrf.py @@ -28,9 +28,15 @@ REASON_BAD_ORIGIN = "Origin checking failed - %s does not match any trusted orig REASON_NO_REFERER = "Referer checking failed - no Referer." REASON_BAD_REFERER = "Referer checking failed - %s does not match any trusted origins." REASON_NO_CSRF_COOKIE = "CSRF cookie not set." -REASON_BAD_TOKEN = "CSRF token missing or incorrect." +REASON_CSRF_TOKEN_INCORRECT = 'CSRF token incorrect.' +REASON_CSRF_TOKEN_MISSING = 'CSRF token missing.' REASON_MALFORMED_REFERER = "Referer checking failed - Referer is malformed." REASON_INSECURE_REFERER = "Referer checking failed - Referer is insecure while host is secure." +# The reason strings below are for passing to InvalidTokenFormat. They are +# phrases without a subject because they can be in reference to either the CSRF +# cookie or non-cookie token. +REASON_INCORRECT_LENGTH = 'has incorrect length' +REASON_INVALID_CHARACTERS = 'has invalid characters' CSRF_SECRET_LENGTH = 32 CSRF_TOKEN_LENGTH = 2 * CSRF_SECRET_LENGTH @@ -107,13 +113,18 @@ def rotate_token(request): request.csrf_cookie_needs_reset = True +class InvalidTokenFormat(Exception): + def __init__(self, reason): + self.reason = reason + + def _sanitize_token(token): + if len(token) not in (CSRF_TOKEN_LENGTH, CSRF_SECRET_LENGTH): + raise InvalidTokenFormat(REASON_INCORRECT_LENGTH) # Make sure all characters are in CSRF_ALLOWED_CHARS. if invalid_token_chars_re.search(token): - return _get_new_csrf_token() - elif len(token) == CSRF_TOKEN_LENGTH: - return token - elif len(token) == CSRF_SECRET_LENGTH: + raise InvalidTokenFormat(REASON_INVALID_CHARACTERS) + if len(token) == CSRF_SECRET_LENGTH: # Older Django versions set cookies to values of CSRF_SECRET_LENGTH # alphanumeric characters. For backwards compatibility, accept # such values as unmasked secrets. @@ -121,7 +132,7 @@ def _sanitize_token(token): # different code paths in the checks, although that might be a tad more # efficient. return _mask_cipher_secret(token) - return _get_new_csrf_token() + return token def _compare_masked_tokens(request_csrf_token, csrf_token): @@ -206,7 +217,11 @@ class CsrfViewMiddleware(MiddlewareMixin): except KeyError: return None - csrf_token = _sanitize_token(cookie_token) + try: + csrf_token = _sanitize_token(cookie_token) + except InvalidTokenFormat: + csrf_token = _get_new_csrf_token() + if csrf_token != cookie_token: # Cookie token needed to be replaced; # the cookie needs to be reset. @@ -381,11 +396,18 @@ class CsrfViewMiddleware(MiddlewareMixin): if request_csrf_token == '': # Fall back to X-CSRFToken, to make things easier for AJAX, and # possible for PUT/DELETE. - request_csrf_token = request.META.get(settings.CSRF_HEADER_NAME, '') + try: + request_csrf_token = request.META[settings.CSRF_HEADER_NAME] + except KeyError: + return self._reject(request, REASON_CSRF_TOKEN_MISSING) + + try: + request_csrf_token = _sanitize_token(request_csrf_token) + except InvalidTokenFormat as exc: + return self._reject(request, f'CSRF token {exc.reason}.') - request_csrf_token = _sanitize_token(request_csrf_token) if not _compare_masked_tokens(request_csrf_token, csrf_token): - return self._reject(request, REASON_BAD_TOKEN) + return self._reject(request, REASON_CSRF_TOKEN_INCORRECT) return self._accept(request) diff --git a/tests/csrf_tests/tests.py b/tests/csrf_tests/tests.py index 51286d0fb4..c326ba2410 100644 --- a/tests/csrf_tests/tests.py +++ b/tests/csrf_tests/tests.py @@ -5,9 +5,9 @@ from django.contrib.sessions.backends.cache import SessionStore from django.core.exceptions import ImproperlyConfigured from django.http import HttpRequest, HttpResponse from django.middleware.csrf import ( - CSRF_SESSION_KEY, CSRF_TOKEN_LENGTH, REASON_BAD_ORIGIN, REASON_BAD_TOKEN, - REASON_NO_CSRF_COOKIE, CsrfViewMiddleware, RejectRequest, - _compare_masked_tokens as equivalent_tokens, get_token, + CSRF_SESSION_KEY, CSRF_TOKEN_LENGTH, REASON_BAD_ORIGIN, + REASON_CSRF_TOKEN_MISSING, REASON_NO_CSRF_COOKIE, CsrfViewMiddleware, + RejectRequest, _compare_masked_tokens as equivalent_tokens, get_token, ) from django.test import SimpleTestCase, override_settings from django.views.decorators.csrf import csrf_exempt, requires_csrf_token @@ -125,28 +125,28 @@ class CsrfViewMiddlewareTestMixin: If a CSRF cookie is present but with no token, the middleware rejects the incoming request. """ - self._check_bad_or_missing_token(None, REASON_BAD_TOKEN) + self._check_bad_or_missing_token(None, REASON_CSRF_TOKEN_MISSING) def test_csrf_cookie_bad_token_characters(self): """ If a CSRF cookie is present but the token has invalid characters, the middleware rejects the incoming request. """ - self._check_bad_or_missing_token(64 * '*', REASON_BAD_TOKEN) + self._check_bad_or_missing_token(64 * '*', 'CSRF token has invalid characters.') def test_csrf_cookie_bad_token_length(self): """ If a CSRF cookie is present but the token has an incorrect length, the middleware rejects the incoming request. """ - self._check_bad_or_missing_token(16 * 'a', REASON_BAD_TOKEN) + self._check_bad_or_missing_token(16 * 'a', 'CSRF token has incorrect length.') def test_csrf_cookie_incorrect_token(self): """ If a CSRF cookie is present but the correctly formatted token is incorrect, the middleware rejects the incoming request. """ - self._check_bad_or_missing_token(64 * 'a', REASON_BAD_TOKEN) + self._check_bad_or_missing_token(64 * 'a', 'CSRF token incorrect.') def test_process_request_csrf_cookie_and_token(self): """ @@ -601,7 +601,10 @@ class CsrfViewMiddlewareTestMixin: with self.assertLogs('django.security.csrf', 'WARNING') as cm: resp = mw.process_view(req, post_form_view, (), {}) self.assertEqual(resp.status_code, 403) - self.assertEqual(cm.records[0].getMessage(), 'Forbidden (%s): ' % REASON_BAD_TOKEN) + self.assertEqual( + cm.records[0].getMessage(), + 'Forbidden (%s): ' % REASON_CSRF_TOKEN_MISSING, + ) @override_settings(ALLOWED_HOSTS=['www.example.com']) def test_bad_origin_bad_domain(self):