Opened 9 years ago
Closed 9 years ago
#26146 closed Cleanup/optimization (wontfix)
CsrfViewMiddleware responds to any arbitrary HTTP method outside of 'GET', 'HEAD', 'OPTIONS', and 'TRACE'
Reported by: | Kyle Baker | Owned by: | nobody |
---|---|---|---|
Component: | CSRF | Version: | 1.8 |
Severity: | Normal | Keywords: | trace, track, csrf, error, page |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Not sure if this is desired behavior or not, but we had a security audit on our website. They found that when a TRACK request was made, they hit a django error page. TRACK seems to be an obscure variant of TRACE unique to some Microsoft systems. In our case, they were hitting a CSRF failure page. We don't want people to see anything about us using csrf tokens, or about the fact that we're using django (or anything else about our security measures or server environment), so we've just overridden our CSRF_FAILURE_VIEW to be a simple plain text 401 in our settings when DEBUG is False.
I can't help but wonder, though... Why does Django respond to this verb in the first place? It's a non-standard verb. TRACE is all that is needed to comply with HTTP standards. It really is more of a minor nuisance that it introduces a (tiny) security issue, with dubious gain. Most developers have never heard of TRACK and wouldn't know to do something about it.
Thoughts?
Change History (7)
comment:1 by , 9 years ago
Component: | HTTP handling → CSRF |
---|---|
Summary: | django responds to obscure non-standard HTTP verb TRACK? → CsrfViewMiddleware responds to any arbitrary HTTP method outside of 'GET', 'HEAD', 'OPTIONS', and 'TRACE' |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 9 years ago
ISTM that the behavior of the CSRF middleware there is fine; it _should_ err on the side of being cautious, and assume that any HTTP verb it doesn't know for sure is in the "safe" list must be "unsafe" and need CSRF protection.
The question is whether something _else_ in Django should validate that the incoming request method is within a list of known methods (and reject it with 405 METHOD NOT ALLOWED if not?) The OP doesn't clarify what behavior they expected here.
Anything we would do on that score needs to be opt-in, I think; right now AFAIK nothing prevents someone from using Django with custom non-standard HTTP verbs, and we shouldn't just break that.
Personally I'm not sure there's anything here that needs fixing, but I guess we could add some kind of ValidateKnownHttpMethod
middleware that people can use if they want, or something.
comment:3 by , 9 years ago
My first thought would be to simply do something like (please take this more as pseudo-code than actual python):
if request.method not in ('GET', 'HEAD', 'OPTIONS', 'TRACE'): if passes_check(): ok() elif request.method in ('PUT', 'POST', 'DELETE'): reject() else: # Unknown method that failed the check return 405 (or maybe 400?)
Would this be deemed acceptable?
Personally I'm not sure there's anything here that needs fixing, but I guess we could add some kind of ValidateKnownHttpMethod middleware that people can use if they want, or something.
The problem is that that would somewhat clash with what .dispatch()
does right now.
comment:4 by , 9 years ago
I don't see why it's the CSRF middleware's responsibility to return a 405 or 400 in case of an HTTP verb it doesn't recognize. That seems out of scope for the CSRF middleware. It should exempt known-safe verbs, and assume everything else needs protection. Which is exactly what it does now.
If you want unknown verbs to be unconditionally rejected with a 405, just write a middleware to do that.
I don't see how a new middleware rejecting unknown verbs with 405 would "clash" with .dispatch()
any more than having CSRF middleware reject them with 405 would.
I still don't see anything that needs changing here.
comment:5 by , 9 years ago
I'm inclined to close this ticket; Tim, can you comment on what kind of change you were envisioning when you accepted it?
comment:6 by , 9 years ago
I was concerned that you can identify a Django-powered site (presuming it hasn't customized settings.CSRF_FAILURE_VIEW
) using something like curl -X TRACK https://www.djangoproject.com/
(which returns a "CSRF verification failed" page), however, now I realized the same issue applies to POST
or any other verb so that concern isn't really related this ticket.
comment:7 by , 9 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Yeah, if you want to obscure the fact that you're using Django you need to customize the CSRF failure view, for sure.
Closing this as wontfix.
The issue seems to be this list here: https://github.com/django/django/blob/31817dd2eb69db54eb559716aae42fe55ada5fea/django/middleware/csrf.py#L127
I agree this doesn't seem ideal.