DisallowedHost can cause uncaught exception and HTTP 500

[Greg Price]

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.

[Tomer Chachamu]

There would need to be a deprecation path for custom 400 views that are using csrf tokens, so I think catching DisallowedHost in the traceback is the right answer here.

[Tomer Chachamu]

​PR

[Tim Graham <timograham@…>]

In 7ec0fdf6:

Fixed #28693 -- Fixed crash in CsrfViewMiddleware when an HTTPS request has an invalid host.