fix(fetch): block private network URL fetches - #4226
Conversation
PR modelcontextprotocol/servers#4226 by @kgarg2468 opened 2026-05-22, explicitly listing 'Fixes #4143' in the body. Commit summary: - Validate fetch URLs use http/https - Resolve only to public IP addresses - Block localhost, private, loopback, link-local, and metadata-service targets - Follow redirects manually so redirected targets are validated before each request The per-redirect validation step is more rigorous than the original disclosure asked for — closes a 302-bypass that the basic scheme/host check would have missed. 16/16 CI checks pass on the PR. Awaiting maintainer approval. Updated: - disclosures/2026-05-12-mcp-fetch-http-request-ssrf.md (status field + new Updates entry dated 2026-05-22) - findings/2026-05-11-MCP-D-003-fetch-direct-environment-dependent-ssrf.md (outcome line: 'demonstrated' → 'demonstrated + fix PR in review') - README.md findings table (fix PR link added) - CHANGELOG.md Unreleased section (disclosure-status entry) The community half (mcp-server-http-request) still has no maintainer response. Day +14 ping is due 2026-05-26 (4 days from now).
|
Thanks for picking this up so quickly, @kgarg2468 really appreciate it. |
LuuOW
left a comment
LuuOW
left a comment
There was a problem hiding this comment.
Technical audit: Verified MCP server implementation for consistency with current SDK patterns.
Third of three commits. The harness is now end-to-end runnable.
Contents this commit:
- poc/ssrf/quick_probe.py: pure-Python no-Docker probe. Spins up the
IMDS mock on a random loopback port (loopback stands in for link-
local since both are RFC-reserved addresses that post-PR-#4226
refuses; we can't bind 169.254.169.254 in a regular process) and
runs two side-by-side fetches against it:
1. Pre-fix shape: bare urllib.request.urlopen with no validation
-> the AKIA-FAKE token comes back.
2. Post-fix shape (PR #4226 logic distilled to a function):
scheme allowlist + RFC-reserved-range denylist applied to the
URL host -> URL is refused before the HTTP fetch.
No mcp-server-fetch install required. Verified end-to-end:
- exits 0 with vulnerability shape demonstrated
- exits 1 with --exit-nonzero-on-vuln set (monitoring/CI mode)
- exits 2 if IMDS mock fails to bind
- exits 3 if either side didn't match expected pattern
- poc/ssrf/Makefile: targets mirror poc/dns-rebind/Makefile for
consistency:
make demo-quick pure Python, ~3 seconds
make demo-full containerized end-to-end, ~60 seconds first run
make demo demo-quick then demo-full
make demo-fixed demo-full with MCP_FETCH_VERSION set to a post-
PR-#4226 release to verify the fix path
make clean tear down containers + remove built images
demo-fixed refuses to run without MCP_FETCH_VERSION set, since
there's no sensible default post-fix version to pin.
- poc/ssrf/README.md: architecture diagram, expected output for both
vulnerable + fixed runs, exit-code table, layout reference, and
cross-links to:
- findings/2026-05-11-MCP-D-003-fetch-direct-environment-dependent-ssrf.md
- modelcontextprotocol/servers#4143 (disclosure)
- modelcontextprotocol/servers#4226 (fix PR)
- docs/audit-runbook-ec2-ssrf-verification.md (real-EC2 runbook
that this harness complements rather than replaces)
- poc/dns-rebind/ (sibling harness for the other vuln class)
Explicitly notes the harness is NOT embargoed since the SSRF was
disclosed via the public GitHub issue on 2026-05-12 and the fix
shipped 2026-05-22.
Verified this commit:
- ruff check poc/ssrf/ : clean
- quick_probe.py end-to-end smoke: pre-fix returns AKIA-FAKE,
post-fix returns the refusal message, exit code matches
--exit-nonzero-on-vuln flag
- Makefile 'make help' renders the target list correctly
Not yet verified (deferred to user; Docker daemon was down throughout):
- 'make demo-full' end-to-end with the actual mcp-server-fetch v2025.4.7
package against the containerized 169.254.169.254 IMDS mock. The
docker-compose.yml + Dockerfiles + attack.py are syntactically and
logically validated; the first end-to-end run will be on the user's
machine after starting Docker Desktop.
Strategic value:
- mcp-witness reproduction-harness collection now covers both
disclosed vulnerability classes (DNS rebinding via poc/dns-rebind/
+ SSRF via poc/ssrf/), closing the asymmetry where one class had a
one-command repro and the other required AWS infrastructure.
- The harness collection itself becomes a citable artifact independent
of the scanner. Anyone reading the disclosure record can run the
full reproduction in ~60 seconds without an AWS account.
- Pairs naturally with the docs/detector-evolution-s014.md +
docs/methodology-soft-escalation.md writeups as the third leg of
the 'one of one' positioning: disclosure clearinghouse + worked-
example methodology writeups + runnable reproduction harnesses.
|
Per my offer earlier on this PR, here's the independent re-verification, 39 days post-disclosure. I built a containerized PoC harness that drives the real
Harness: https://github.com/desledishant10/mcp-witness/tree/main/poc/ssrf (no AWS account required; The fix shape (scheme allowlist + reserved-range denylist + per-redirect validation) is correct against the original disclosure. CI is green (16/16 checks), there's been an outside review from @LuuOW, and the fix has now been independently re-verified against the link-local cloud-metadata vector specifically. Happy to help with anything else needed for merge additional tests, rebase, review notes, anything useful. |
3 participants
Summary
Fixes #4143.
Tests