Fixed #20869 -- made CSRF tokens change every request by salt-encrypting them

Note that the cookie is not changed every request, just the token retrieved
by the `get_token()` method (used also by the `{% csrf_token %}` tag).

While at it, made token validation strict: Where, before, any length was
accepted and non-ASCII chars were ignored, we now treat anything other than
`[A-Za-z0-9]{64}` as invalid (except for 32-char tokens, which, for
backwards-compatibility, are accepted and replaced by 64-char ones).

Thanks Trac user patrys for reporting, github user adambrenecki
for initial patch, Tim Graham for help, and Curtis Maloney,
Collin Anderson, Florian Apolloner, Markus Holtermann & Jon Dufresne
for reviews.

3 files changed, 104 insertions, 19 deletions

diff --git a/tests/csrf_tests/test_context_processor.py b/tests/csrf_tests/test_context_processor.py
index 270b3e4771..5db0116db0 100644
--- a/tests/csrf_tests/test_context_processor.py
+++ b/tests/csrf_tests/test_context_processor.py
@@ -1,6 +1,5 @@
-import json
-
 from django.http import HttpRequest
+from django.middleware.csrf import _compare_salted_tokens as equivalent_tokens
 from django.template.context_processors import csrf
 from django.test import SimpleTestCase
 from django.utils.encoding import force_text
@@ -10,6 +9,7 @@ class TestContextProcessor(SimpleTestCase):
 
     def test_force_text_on_token(self):
         request = HttpRequest()
-        request.META['CSRF_COOKIE'] = 'test-token'
+        test_token = '1bcdefghij2bcdefghij3bcdefghij4bcdefghij5bcdefghij6bcdefghijABCD'
+        request.META['CSRF_COOKIE'] = test_token
         token = csrf(request).get('csrf_token')
-        self.assertEqual(json.dumps(force_text(token)), '"test-token"')
+        self.assertTrue(equivalent_tokens(force_text(token), test_token))
diff --git a/tests/csrf_tests/tests.py b/tests/csrf_tests/tests.py
index 54570dca42..5ed7c9dc3d 100644
--- a/tests/csrf_tests/tests.py
+++ b/tests/csrf_tests/tests.py
@@ -2,15 +2,20 @@
 from __future__ import unicode_literals
 
 import logging
+import re
+import warnings
 
 from django.conf import settings
 from django.http import HttpRequest, HttpResponse
 from django.middleware.csrf import (
-    CSRF_KEY_LENGTH, CsrfViewMiddleware, get_token,
+    CSRF_TOKEN_LENGTH, CsrfViewMiddleware,
+    _compare_salted_tokens as equivalent_tokens, get_token,
 )
 from django.template import RequestContext, Template
 from django.template.context_processors import csrf
 from django.test import SimpleTestCase, override_settings
+from django.utils.encoding import force_bytes
+from django.utils.six import text_type
 from django.views.decorators.csrf import (
     csrf_exempt, ensure_csrf_cookie, requires_csrf_token,
 )
@@ -57,10 +62,7 @@ class TestingHttpRequest(HttpRequest):
 
 
 class CsrfViewMiddlewareTest(SimpleTestCase):
-    # The csrf token is potentially from an untrusted source, so could have
-    # characters that need dealing with.
-    _csrf_id_cookie = b"<1>\xc2\xa1"
-    _csrf_id = "1"
+    _csrf_id = _csrf_id_cookie = '1bcdefghij2bcdefghij3bcdefghij4bcdefghij5bcdefghij6bcdefghijABCD'
 
     def _get_GET_no_csrf_cookie_request(self):
         return TestingHttpRequest()
@@ -85,8 +87,24 @@ class CsrfViewMiddlewareTest(SimpleTestCase):
         req.POST['csrfmiddlewaretoken'] = self._csrf_id
         return req
 
+    def _get_POST_bare_secret_csrf_cookie_request(self):
+        req = self._get_POST_no_csrf_cookie_request()
+        req.COOKIES[settings.CSRF_COOKIE_NAME] = self._csrf_id_cookie[:32]
+        return req
+
+    def _get_POST_bare_secret_csrf_cookie_request_with_token(self):
+        req = self._get_POST_bare_secret_csrf_cookie_request()
+        req.POST['csrfmiddlewaretoken'] = self._csrf_id_cookie[:32]
+        return req
+
     def _check_token_present(self, response, csrf_id=None):
-        self.assertContains(response, "name='csrfmiddlewaretoken' value='%s'" % (csrf_id or self._csrf_id))
+        text = text_type(response.content, response.charset)
+        match = re.search("name='csrfmiddlewaretoken' value='(.*?)'", text)
+        csrf_token = csrf_id or self._csrf_id
+        self.assertTrue(
+            match and equivalent_tokens(csrf_token, match.group(1)),
+            "Could not find csrfmiddlewaretoken to match %s" % csrf_token
+        )
 
     def test_process_view_token_too_long(self):
         """
@@ -94,12 +112,45 @@ class CsrfViewMiddlewareTest(SimpleTestCase):
         created.
         """
         req = self._get_GET_no_csrf_cookie_request()
-        req.COOKIES[settings.CSRF_COOKIE_NAME] = 'x' * 10000000
+        req.COOKIES[settings.CSRF_COOKIE_NAME] = 'x' * 100000
         CsrfViewMiddleware().process_view(req, token_view, (), {})
         resp = token_view(req)
         resp2 = CsrfViewMiddleware().process_response(req, resp)
         csrf_cookie = resp2.cookies.get(settings.CSRF_COOKIE_NAME, False)
-        self.assertEqual(len(csrf_cookie.value), CSRF_KEY_LENGTH)
+        self.assertEqual(len(csrf_cookie.value), CSRF_TOKEN_LENGTH)
+
+    def test_process_view_token_invalid_chars(self):
+        """
+        If the token contains non-alphanumeric characters, it is ignored and a
+        new token is created.
+        """
+        token = ('!@#' + self._csrf_id)[:CSRF_TOKEN_LENGTH]
+        req = self._get_GET_no_csrf_cookie_request()
+        req.COOKIES[settings.CSRF_COOKIE_NAME] = token
+        CsrfViewMiddleware().process_view(req, token_view, (), {})
+        resp = token_view(req)
+        resp2 = CsrfViewMiddleware().process_response(req, resp)
+        csrf_cookie = resp2.cookies.get(settings.CSRF_COOKIE_NAME, False)
+        self.assertEqual(len(csrf_cookie.value), CSRF_TOKEN_LENGTH)
+        self.assertNotEqual(csrf_cookie.value, token)
+
+    def test_process_view_token_invalid_bytes(self):
+        """
+        If the token contains improperly encoded unicode, it is ignored and a
+        new token is created.
+        """
+        token = (b"<1>\xc2\xa1" + force_bytes(self._csrf_id, 'ascii'))[:CSRF_TOKEN_LENGTH]
+        req = self._get_GET_no_csrf_cookie_request()
+        req.COOKIES[settings.CSRF_COOKIE_NAME] = token
+        # We expect a UnicodeWarning here, because we used broken utf-8 on purpose
+        with warnings.catch_warnings():
+            warnings.filterwarnings("ignore", category=UnicodeWarning)
+            CsrfViewMiddleware().process_view(req, token_view, (), {})
+        resp = token_view(req)
+        resp2 = CsrfViewMiddleware().process_response(req, resp)
+        csrf_cookie = resp2.cookies.get(settings.CSRF_COOKIE_NAME, False)
+        self.assertEqual(len(csrf_cookie.value), CSRF_TOKEN_LENGTH)
+        self.assertNotEqual(csrf_cookie.value, token)
 
     def test_process_response_get_token_used(self):
         """
@@ -295,6 +346,37 @@ class CsrfViewMiddlewareTest(SimpleTestCase):
         csrf_cookie = resp2.cookies[settings.CSRF_COOKIE_NAME]
         self._check_token_present(resp, csrf_id=csrf_cookie.value)
 
+    def test_cookie_not_reset_on_accepted_request(self):
+        """
+        The csrf token used in posts is changed on every request (although
+        stays equivalent). The csrf cookie should not change on accepted
+        requests. If it appears in the response, it should keep its value.
+        """
+        req = self._get_POST_request_with_token()
+        CsrfViewMiddleware().process_view(req, token_view, (), {})
+        resp = token_view(req)
+        resp = CsrfViewMiddleware().process_response(req, resp)
+        csrf_cookie = resp.cookies.get(settings.CSRF_COOKIE_NAME, None)
+        if csrf_cookie:
+            self.assertEqual(
+                csrf_cookie.value, self._csrf_id_cookie,
+                "CSRF cookie was changed on an accepted request"
+            )
+
+    def test_bare_secret_accepted_and_replaced(self):
+        """
+        The csrf token is reset from a bare secret.
+        """
+        req = self._get_POST_bare_secret_csrf_cookie_request_with_token()
+        req2 = CsrfViewMiddleware().process_view(req, token_view, (), {})
+        self.assertIsNone(req2)
+        resp = token_view(req)
+        resp = CsrfViewMiddleware().process_response(req, resp)
+        self.assertIn(settings.CSRF_COOKIE_NAME, resp.cookies, "Cookie was not reset from bare secret")
+        csrf_cookie = resp.cookies[settings.CSRF_COOKIE_NAME]
+        self.assertEqual(len(csrf_cookie.value), CSRF_TOKEN_LENGTH)
+        self._check_token_present(resp, csrf_id=csrf_cookie.value)
+
     @override_settings(DEBUG=True)
     def test_https_bad_referer(self):
         """
@@ -592,7 +674,7 @@ class CsrfViewMiddlewareTest(SimpleTestCase):
 
             POST = property(_get_post, _set_post)
 
-        token = 'ABC'
+        token = ('ABC' + self._csrf_id)[:CSRF_TOKEN_LENGTH]
 
         req = CsrfPostRequest(token, raise_error=False)
         resp = CsrfViewMiddleware().process_view(req, post_form_view, (), {})
diff --git a/tests/template_backends/test_dummy.py b/tests/template_backends/test_dummy.py
index e6e60e71a2..83b42c7eb4 100644
--- a/tests/template_backends/test_dummy.py
+++ b/tests/template_backends/test_dummy.py
@@ -2,9 +2,13 @@
 
 from __future__ import unicode_literals
 
+import re
+
 from django.forms import CharField, Form, Media
 from django.http import HttpRequest
-from django.middleware.csrf import CsrfViewMiddleware, get_token
+from django.middleware.csrf import (
+    CsrfViewMiddleware, _compare_salted_tokens as equivalent_tokens, get_token,
+)
 from django.template import TemplateDoesNotExist, TemplateSyntaxError
 from django.template.backends.dummy import TemplateStrings
 from django.test import SimpleTestCase
@@ -81,11 +85,10 @@ class TemplateStringsTests(SimpleTestCase):
         template = self.engine.get_template('template_backends/csrf.html')
         content = template.render(request=request)
 
-        expected = (
-            '<input type="hidden" name="csrfmiddlewaretoken" '
-            'value="{}" />'.format(get_token(request)))
-
-        self.assertHTMLEqual(content, expected)
+        expected = '<input type="hidden" name="csrfmiddlewaretoken" value="([^"]+)" />'
+        match = re.match(expected, content) or re.match(expected.replace('"', "'"), content)
+        self.assertTrue(match, "hidden csrftoken field not found in output")
+        self.assertTrue(equivalent_tokens(match.group(1), get_token(request)))
 
     def test_no_directory_traversal(self):
         with self.assertRaises(TemplateDoesNotExist):