id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
32795	Reject requests earlier if the non-cookie CSRF token is missing or has the wrong format	Chris Jerdonek	Chris Jerdonek	"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.)
"	Cleanup/optimization	closed	CSRF	dev	Normal	fixed		Shai Berger Florian Apolloner	Ready for checkin	1	0	0	0	0	0