fix(mcp): validate the Host header, not only Origin (#50) - #55
Merged
Conversation
Origin validation (#45, shipped in v2026.08.31) does not stop DNS rebinding, and the comment on #50 is right about why. Once evil.example makes rebind.evil.example resolve to 127.0.0.1, the browser treats the call as same-origin: the request carries Host: rebind.evil.example and either a matching Origin or none at all. Measured against that build, both shapes were answered 200 — the first passing ClassifyOrigin's same-origin rule, which proves nothing when the attacker owns both sides of the comparison, the second passing the no-Origin rule. The Host header is the half an attacker cannot launder, so it is now classified on its own, before Origin. Loopback names and IP literals pass unconditionally (an IP address has no name to rebind, which is what keeps --mcp-host 0.0.0.0 reachable at a LAN address with no config), as does the address bound and anything named by the new --allowed-hosts. Passing --allowed-hosts is what turns refusal on. Without it an unrecognised Host is still served, with one warning per distinct host — so a deployment reached through a DNS name or a reverse proxy does not break on upgrade. That leaves the rebinding path open by default, which is a deliberate trade-off and the reason #50 does not close here.
TestPort()+30..32 already belong to the HTTP status-conformance tests, so the new cases were binding ports a neighbour had just used. Windows CI crashed in a nearby test because of it.
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.
Closes nothing on its own — see the note at the bottom about #50.
The gap
PR #45 removed the wildcard
Access-Control-Allow-Originand added Origin validation, and that part works:Origin: https://evil.exampleis answered 403. The comment on #50 argues that Host must be validated too, and it is right. Measured against the released build:Both are what DNS rebinding looks like once
evil.examplepointsrebind.evil.exampleat127.0.0.1. The browser considers the call same-origin, so the first request passesClassifyOrigin's same-origin rule — which proves nothing, because after rebinding the attacker owns both halves of that comparison — and the second passes the no-Origin rule. Behind the endpoint sitadt_write_source,adt_delete_object,adt_activate,adt_release_transport,bw_delete_object.What this does
ClassifyHostclassifies theHostheader on its own, checked before Origin:NoHost— browsers always send onelocalhost,127.0.0.1,[::1]Loopback192.168.1.5,[fe80::1])IpLiteral— an IP address has no DNS name to rebind--allowed-hostsAllowlisted--allowed-hosts '*'WildcardUnrecognised--allowed-hosts <list>on bothmcp --httpandcatalog webui. TheIpLiteralrule is what keeps--host 0.0.0.0reachable at a LAN address with no configuration.Warn-only by default
Passing
--allowed-hostsis what turns refusal on. Without it anUnrecognisedHost is still served, with one warning per distinct host on stderr (deduplicated, so a scripted attack cannot flood the log). A deployment reached through a DNS name or a reverse proxy therefore does not break on upgrade.The trade-off to be explicit about: in the default configuration the rebinding path above is still open. That is why this PR does not close #50 — closing it would mean flipping the default to enforcing, which is a separate decision.
Verified live
Plus: 1363 unit tests pass (13 new), and
scripts/ci/webui_smoke.pypasses — the embedded web UI is same-origin on loopback, so enforcement costs it nothing.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.