#32795: Reject requests earlier if the CSRF token is missing or has the wrong
format
------------------------------------------------+------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: CSRF | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
I noticed that `CsrfViewMiddleware.process_view()` does what seems like
unnecessary work when the non-cookie CSRF token is either missing or has
the wrong format (i.e. has the wrong length or contains characters that
aren't allowed). Specifically, in
[https://github.com/django/django/blob/b746596f5f0e1fcac791b0f7c8bfc3d69dfef2ff/django/middleware/csrf.py#L385-L386
these lines]:
{{{#!python
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, '')
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)
}}}
if the `request_csrf_token` is missing or has the wrong format, the code
will proceed inside `_sanitize_token ()` to use Python's `secrets` module
twice to generate both a new token and a mask for the token, but only for
the purposes of calling `_compare_masked_tokens()` in a way that will be
guaranteed to fail (since the token being passed will be brand new). And
then it will call `_compare_masked_tokens()` with that value.
However, if the non-cookie token is missing or has the wrong format, it
seems like the request can be rejected at that point outright without
needing to do the work above. It doesn't seem like rejecting the request
outright will reveal any sensitive information since the correct token
length and allowed characters aren't secret information. (Django's
security model assumes that information is publicly known.)
Another advantage of rejecting earlier is that the failure message can be
more specific. Namely, instead of just using `REASON_BAD_TOKEN` ("CSRF
token missing or incorrect"), more specific messages can be used like
"CSRF token missing," "CSRF token has wrong length," and "CSRF token
contains invalid characters." That could be useful in troubleshooting CSRF
issues, which can sometimes be hard to troubleshoot.
A third advantage is that this will make the code easier to understand.
This is because currently, it's hard to tell whether calling
`_sanitize_token()` and `_compare_masked_tokens()` are actually needed for
security reasons even when the CSRF token is missing or has the wrong
format. (There currently aren't any comments explaining why it's needed if
in fact it is.)
--
Ticket URL: <https://code.djangoproject.com/ticket/32795>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/052.8c906cd9b293374d7d8cf961e77488e9%40djangoproject.com.
