#30091 closed Cleanup/optimization (fixed)
Incorrect middleware ordering allows invalid HTTP_HOST header to cause CsrfViewMiddleware failure when using CSRF_USE_SESSIONS.
Reported by: | Mark Gregson | Owned by: | Carlton Gibson |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Normal | Keywords: | middleware |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I've observed that when using CSRF_USE_SESSIONS = True and with SessionMiddleware correctly placed before CsrfViewMiddleware that a request with an invalid HTTP_HOST header raises an exception and, while preparing a response for this exception, the CsrfViewMiddleware raises an unhandled ImproperlyConfigured exception and an internal server error is returned to the browser.
I expect that an HTTP 400 will be returned to the user for an invalid HTTP_HOST header and that the CsrfViewMiddleware will not raise an exception.
To reproduce, configure the application as above, set up a new hostname (one that is not in ALLOWED_HOSTS) pointing at the Django application (using the hosts file, for example), and then request that hostname.
Attachments (2)
Change History (11)
by , 6 years ago
Attachment: | error_log.txt added |
---|
comment:1 by , 6 years ago
comment:2 by , 6 years ago
Hi Carlton. Not one I can easily share. I'll see if I can create a minimal project that reproduces it.
by , 6 years ago
Attachment: | min_example.diff added |
---|
Patch to create minimal project reproducing the bug
comment:3 by , 6 years ago
Okay, I've just attached a minimal working example. I've provided it as a patch to a 'django-admin startproject' site template using Django 1.11.18. You should just be able to do:
- django-admin startproject mysite
- cd mysite
- patch -p1 < min_example.diff
In this example I've configured ALLOWED_HOSTS = ['localhost'] and added '127.0.0.1 dummy' to my /etc/hosts file. Requesting http://dummy:8000/ reproduces the ImproperlyConfigured exception.
While producing this example I found that placing CommonMiddleware before SessionMiddleware was also required to reproduce the bug. I'm unsure if this is a configuration error; but I couldn't see anything about ordering of those two middleware classes when I scanned the documentation just now.
Cheers
Mark
comment:4 by , 6 years ago
Hey Mark. Super effort. Thank you. I’ll have a look at the project and get back to you. 👍
comment:5 by , 6 years ago
Component: | Uncategorized → Documentation |
---|---|
Has patch: | set |
Keywords: | middleware added |
Owner: | changed from | to
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
Version: | 1.11 → master |
OK, this is a documentation issue.
Error views are wrapped with @requires_csrf_token
, so SessionMiddleware
must appear before any middleware that may raise an exception when using CSRF_USE_SESSIONS
.
The default project template has this correct, so you have to "opt-in" to this error.
Thanks for the report and the project to reproduce Mark!
comment:6 by , 6 years ago
Summary: | Invalid HTTP_HOST header causes CsrfViewMiddleware failure → Incorrect middleware ordering allows invalid HTTP_HOST header to cause CsrfViewMiddleware failure when using CSRF_USE_SESSIONS. |
---|
Hi Mark. Do you have an example project you can upload demonstrating this issue? Thanks!