From: Ben Darnell Date: Wed, 10 Dec 2025 20:15:25 +0000 (-0500) Subject: web: Harden against invalid HTTP reason phrases X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f3b99cd34d4c6360f0db34b3c39f700c002b1415;p=thirdparty%2Ftornado.git web: Harden against invalid HTTP reason phrases We allow applications to set custom reason phrases for the HTTP status line (to support custom status codes), but if this were exposed to untrusted data it could be exploited in various ways. This commit guards against invalid reason phrases in both HTTP headers and in error pages. --- diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py index c18e250e..27df3f7a 100644 --- a/tornado/test/web_test.py +++ b/tornado/test/web_test.py @@ -1769,7 +1769,7 @@ class StatusReasonTest(SimpleHandlerTestCase): class Handler(RequestHandler): def get(self): reason = self.request.arguments.get("reason", []) - self.set_status( + raise HTTPError( int(self.get_argument("code")), reason=to_unicode(reason[0]) if reason else None, ) @@ -1792,6 +1792,19 @@ class StatusReasonTest(SimpleHandlerTestCase): self.assertEqual(response.code, 682) self.assertEqual(response.reason, "Unknown") + def test_header_injection(self): + response = self.fetch("/?code=200&reason=OK%0D%0AX-Injection:injected") + self.assertEqual(response.code, 200) + self.assertEqual(response.reason, "Unknown") + self.assertNotIn("X-Injection", response.headers) + + def test_reason_xss(self): + response = self.fetch("/?code=400&reason=") + self.assertEqual(response.code, 400) + self.assertEqual(response.reason, "Unknown") + self.assertNotIn(b"script", response.body) + self.assertIn(b"Unknown", response.body) + class DateHeaderTest(SimpleHandlerTestCase): class Handler(RequestHandler): diff --git a/tornado/web.py b/tornado/web.py index 2f702d64..2351afdb 100644 --- a/tornado/web.py +++ b/tornado/web.py @@ -359,8 +359,10 @@ class RequestHandler: :arg int status_code: Response status code. :arg str reason: Human-readable reason phrase describing the status - code. If ``None``, it will be filled in from - `http.client.responses` or "Unknown". + code (for example, the "Not Found" in ``HTTP/1.1 404 Not Found``). + Normally determined automatically from `http.client.responses`; this + argument should only be used if you need to use a non-standard + status code. .. versionchanged:: 5.0 @@ -369,6 +371,14 @@ class RequestHandler: """ self._status_code = status_code if reason is not None: + if "<" in reason or not httputil._ABNF.reason_phrase.fullmatch(reason): + # Logically this would be better as an exception, but this method + # is called on error-handling paths that would need some refactoring + # to tolerate internal errors cleanly. + # + # The check for "<" is a defense-in-depth against XSS attacks (we also + # escape the reason when rendering error pages). + reason = "Unknown" self._reason = escape.native_str(reason) else: self._reason = httputil.responses.get(status_code, "Unknown") @@ -1345,7 +1355,8 @@ class RequestHandler: reason = exception.reason self.set_status(status_code, reason=reason) try: - self.write_error(status_code, **kwargs) + if status_code != 304: + self.write_error(status_code, **kwargs) except Exception: app_log.error("Uncaught exception in write_error", exc_info=True) if not self._finished: @@ -1373,7 +1384,7 @@ class RequestHandler: self.finish( "%(code)d: %(message)s" "%(code)d: %(message)s" - % {"code": status_code, "message": self._reason} + % {"code": status_code, "message": escape.xhtml_escape(self._reason)} ) @property @@ -2520,9 +2531,11 @@ class HTTPError(Exception): mode). May contain ``%s``-style placeholders, which will be filled in with remaining positional parameters. :arg str reason: Keyword-only argument. The HTTP "reason" phrase - to pass in the status line along with ``status_code``. Normally + to pass in the status line along with ``status_code`` (for example, + the "Not Found" in ``HTTP/1.1 404 Not Found``). Normally determined automatically from ``status_code``, but can be used - to use a non-standard numeric code. + to use a non-standard numeric code. This is not a general-purpose + error message. """ def __init__(