#28693: DisallowedHost can cause uncaught exception and HTTP 500
-----------------------------------------+--------------------------------
Reporter: Greg Price | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.11
Severity: Normal | Keywords: DisallowedHost
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+--------------------------------
If the client sends an HTTP "Host:" header which isn't allowed, we try to
return a 400 response while logging a DisallowedHost exception. For a
brief time after `SuspiciousOperation` was first introduced, this caused a
500, but that was fixed in #19866 back in 2013.
But now I've been getting exceptions in production recently that are
caused by DisallowedHost. It turns out that there is a case in the
exception-handling path on the way to returning a 400 response that itself
tries to call `request.get_host()`, and so raises a new DisallowedHost
exception. Here's the test I added to my application's test suite, to test
my workaround:
{{{
class ErrorPageTest(TestCase):
def test_bogus_http_host(self):
# This tests that we've successfully worked around a certain bug
in
# Django's exception handling. The enforce_csrf_checks=True,
# secure=True, and HTTP_REFERER with an `https:` scheme are all
# there to get us down just the right path for Django to blow up
# when presented with an HTTP_HOST that's not a valid DNS name.
client = django.test.Client(enforce_csrf_checks=True)
result = client.post('/json/users',
secure=True,
HTTP_REFERER='https://somewhere',
HTTP_HOST='$nonsense')
self.assertEqual(result.status_code, 400)
}}}
The workaround is to override `handler400` with a simple implementation
that just skips the `@requires_csrf_token` from the default
implementation. Like this:
{{{
def handler400(request, exception):
return HttpResponseBadRequest(
'<h1>Bad Request (400)</h1>', content_type='text/html')
}}}
(That version also skips the template lookup, since we haven't provided a
400 template.)
To fix it in Django, it'd be enough to just remove the
`@requires_csrf_token` decorator from `django.views.defaults.bad_request`.
I'm not sure what that might break, though -- I think the case where it
could matter is if an app's `400.html` tries to use `csrf_token`, but I'm
not sure what the failure would look like. I think it could be pretty
reasonable to say that a 400 error page can't use `csrf_token`, so long as
the error that results if someone tries to do that is an appropriate one.
An alternate fix would be to try to make `requires_csrf_token` more
robust. If it's going to be used on a 4xx error page (or at least on 400),
it needs to really truly never raise an exception, no matter how
nonsensical the request the client sent is, because that's where we go
when we know the client has gotten something wrong.
See https://github.com/zulip/zulip/pull/6934 for an example traceback, and
my actual workaround and test code.
--
Ticket URL: <https://code.djangoproject.com/ticket/28693>
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 post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/050.132973432655a24513296d8215695d04%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
