Ignore Host header in favor of absolute-form request-target authority - #496
Open
agu2347 wants to merge 1 commit into
Open
Ignore Host header in favor of absolute-form request-target authority#496agu2347 wants to merge 1 commit into
agu2347 wants to merge 1 commit into
Conversation
RFC 9112 section 3.2.2 requires: "When a proxy receives a request with an absolute-form of request-target, the [recipient] MUST ignore the received Host header field (if any) and instead replace it with the host information of the request-target." waitress's parser already correctly extracted the request-target's own authority into self.proxy_netloc via split_uri(), but never used it to override the Host header -- the client-supplied Host header (if any) was used as-is, regardless of whether it agreed with the request-target. This means a request like: GET http://evil.com/page HTTP/1.1 Host: victim.com was processed with HTTP_HOST/Host effectively "victim.com" (from the header) even though the request-target's own authority explicitly says "evil.com" -- exactly backwards from what RFC 9112 requires. Since applications commonly use the Host header for request routing, access control decisions, or cache keys, this allows a client-supplied Host header to disagree with (and silently override) what the request-line itself specifies, which could enable host-header-based security checks to be bypassed or confused about the actual requested authority. When proxy_netloc is present (i.e. an absolute-form request-target was used), set headers["HOST"] to it, so it correctly reflects the request-target's authority per RFC 9112, regardless of any Host header that may also have been sent. Verified directly against the exact example from the issue: before the fix, headers["HOST"] was "victim.com" for a request whose request-line said "http://evil.com/page" with a "Host: victim.com" header; after the fix, headers["HOST"] is correctly "evil.com". Also verified: normal origin-form requests (the overwhelmingly common case) are completely unaffected; absolute-form requests where the Host header already matches the authority remain correct; an authority including a non-default port is used verbatim; and an absolute-form request with no Host header at all still gets HOST set from the request-target. Updated the existing testProxyGET test, which asserted the old (vulnerable) behavior by expecting no HOST key at all for an absolute-form request without an explicit Host header -- it now asserts HOST is correctly derived from the request-target's own authority. Added a new regression test reproducing the issue's exact scenario (mismatched Host header on an absolute-form request-target). Confirmed the new test fails with the original code (Host header is the attacker-controlled value) and passes with the fix. Ran the full existing test_parser.py suite (78 passed: 77 baseline + 1 new) and the broader project test suite (753 passed, 50 skipped; the 3 remaining failures are pre-existing socket-binding tests unrelated to this change, confirmed identical on a clean checkout of main). Fixes Pylons#467
Member
|
@tseaver Is there a stance the Pylons Project has on LLM-generated contributions? |
Member
|
Nope, there's no AI policy. This would be a good topic to discuss on the Pylons Project Discord server at https://discord.gg/FrnhPkJa4. icalendar has a pretty good AI policy and automation to detect potential AI slop. https://icalendar.readthedocs.io/en/stable/contribute/index.html#artificial-intelligence-policy |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #467.
RFC 9112 section 3.2.2 requires: "When a proxy receives a request with an absolute-form of request-target, the [recipient] MUST ignore the received Host header field (if any) and instead replace it with the host information of the request-target." waitress's parser already correctly extracted the request-target's own authority into
self.proxy_netlocviasplit_uri(), but never used it to override theHostheader -- the client-suppliedHostheader (if any) was used as-is, regardless of whether it agreed with the request-target.This means a request like:
was processed with
HTTP_HOST/Hosteffectively"victim.com"(from the header) even though the request-target's own authority explicitly says"evil.com"-- exactly backwards from what RFC 9112 requires. Since applications commonly use theHostheader for request routing, access control decisions, or cache keys, this allows a client-suppliedHostheader to disagree with (and silently override) what the request-line itself specifies, which could enable host-header-based security checks to be bypassed or confused about the actual requested authority -- exactly the scenario described in the issue.Fix: when
proxy_netlocis present (i.e. an absolute-form request-target was used), setheaders["HOST"]to it, so it correctly reflects the request-target's authority per RFC 9112, regardless of anyHostheader that may also have been sent.Testing: verified directly against the exact example from the issue: before the fix,
headers["HOST"]was"victim.com"for a request whose request-line said"http://evil.com/page"with a"Host: victim.com"header; after the fix,headers["HOST"]is correctly"evil.com". Also verified: normal origin-form requests (the overwhelmingly common case) are completely unaffected; absolute-form requests where theHostheader already matches the authority remain correct; an authority including a non-default port is used verbatim; and an absolute-form request with noHostheader at all still getsHOSTset from the request-target.Updated the existing
testProxyGETtest, which asserted the old (vulnerable) behavior by expecting noHOSTkey at all for an absolute-form request without an explicitHostheader -- it now assertsHOSTis correctly derived from the request-target's own authority. Added a new regression test reproducing the issue's exact scenario (mismatchedHostheader on an absolute-form request-target). I confirmed the new test fails with the original code (Hostheader is the attacker-controlled value) and passes with the fix. Ran the full existingtest_parser.pysuite (78 passed: 77 baseline + 1 new) and the broader project test suite (753 passed, 50 skipped; the 3 remaining failures are pre-existing socket-binding tests unrelated to this change, confirmed identical on a clean checkout ofmain).