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
28693	DisallowedHost can cause uncaught exception and HTTP 500	Greg Price	Tim Graham <timograham@…>	"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.
"	Bug	closed	HTTP handling	1.11	Normal	fixed	DisallowedHost	Tomer Chachamu	Accepted	1	0	0	0	0	0