diff --git a/CHANGES.txt b/CHANGES.txt index c7f32ea4..6138ce5e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -4,6 +4,16 @@ Unreleased Bugfix ~~~~~~ +- When a request uses the absolute-form of request-target (e.g. + ``GET http://example.com/page HTTP/1.1``), waitress now uses that + request-target's own authority as the ``Host``, ignoring any received + ``Host`` header, per RFC 9112 section 3.2.2. Previously, a client-supplied + ``Host`` header that disagreed with the request-target's authority was + used as-is, which could let a request's ``Host``-based routing, access + control, or cache-key handling be driven by a value the request line + itself contradicted. See + https://github.com/Pylons/waitress/issues/467 + - Renamed the HTTP header "Trailers" to "Trailer" to fix a typo and comply with the correct header name as specified in RFC 7230. diff --git a/src/waitress/parser.py b/src/waitress/parser.py index 1af4594d..79d18088 100644 --- a/src/waitress/parser.py +++ b/src/waitress/parser.py @@ -273,6 +273,20 @@ def parse_header(self, header_plus): self.query, self.fragment, ) = split_uri(uri) + + if self.proxy_netloc: + # RFC 9112 sec 3.2.2: "When a proxy receives a request with an + # absolute-form of request-target, the proxy MUST ignore the + # received Host header field (if any) and instead replace it + # with the host information of the request-target." The same + # requirement applies to an origin server receiving such a + # request. Without this, a client could send a request whose + # Host header differs from the request-target's own authority, + # and any code relying on the (untouched) Host header -- for + # routing, access control, or cache keys -- would be acting on + # a value the request-line itself contradicts. See GH #467. + headers["HOST"] = self.proxy_netloc + self.url_scheme = self.adj.url_scheme connection = headers.get("CONNECTION", "") diff --git a/tests/test_parser.py b/tests/test_parser.py index 5f341ae9..ae568fc1 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -716,7 +716,13 @@ def testProxyGET(self): self.assertTrue(parser.completed) self.assertEqual(parser.version, "8.4") self.assertFalse(parser.empty) - self.assertEqual(parser.headers, {"CONTENT_LENGTH": "6"}) + # HOST is derived from the absolute-form request-target's own + # authority (RFC 9112 sec 3.2.2), even though no explicit Host + # header was sent here. See GH #467. + self.assertEqual( + parser.headers, + {"CONTENT_LENGTH": "6", "HOST": "example.com:8080"}, + ) self.assertEqual(parser.path, "/foobar") self.assertEqual(parser.command, "GET") self.assertEqual(parser.proxy_scheme, "https") @@ -725,6 +731,28 @@ def testProxyGET(self): self.assertEqual(parser.query, "") self.assertEqual(parser.get_body_stream().getvalue(), b"Hello.") + def testProxyGETIgnoresMismatchedHostHeader(self): + """ + Regression test for + https://github.com/Pylons/waitress/issues/467 + + Per RFC 9112 sec 3.2.2, an absolute-form request-target's own + authority must be used as the Host, and any Host header + actually received on the wire must be ignored -- even (and + especially) when it disagrees with the request-target, as a + malicious or misconfigured client might send. + """ + data = ( + b"GET http://example.com/page HTTP/1.1\r\n" + b"Host: attacker-controlled.example\r\n" + b"\r\n" + ) + parser = self.parser + self.feed(data) + self.assertTrue(parser.completed) + self.assertEqual(parser.headers["HOST"], "example.com") + self.assertEqual(parser.path, "/page") + def testDuplicateHeaders(self): # Ensure that headers with the same key get concatenated as per # RFC2616.