[Security 1/9] SSRF guardrails for server-side connector fetches - #242
Merged
Conversation
Port the fork's SSRF hardening for MCP connector egress into the upstream layout: - Extract private/reserved IP classification into lib/privateIp.ts and fix IPv6 gaps: fe80::/10 link-local matching (the /^fe[89ab]:/ regex only matched the hextet "fe8:" and let fe80::1 through), hex-form IPv4-mapped addresses (::ffff:a00:1), NAT64 (64:ff9b::/96) and 6to4 (2002::/16) embedded IPv4 ranges. - Strip brackets from IPv6 literals in validateRemoteMcpUrl so [::1] et al. are classified by the private-IP guard instead of falling through to DNS lookup. - Route all OAuth egress (metadata fetch, discovery probes, dynamic client registration, token refresh) through guardedFetch so every outbound MCP request gets the same HTTPS-only / blocked-host / private-IP / no-redirect checks; the discovery probes were previously raw, unvalidated fetches. - Add SSRF regression tests (run atop the test-harness PR) and exclude test files from the tsc production build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEguyEgXa9JjCciXCcVemC
Restore the fork's pinnedGuardAgent verbatim: guardedFetch now routes
through a per-request undici Agent whose connect-time DNS lookup runs
the private-IP guard and returns only validated addresses, so the
address we validate is the address we connect to — closing the
DNS-rebinding/TOCTOU window between the pre-fetch validation lookup and
the socket's own resolution. Adds the undici runtime dependency the
fork ships for exactly this purpose ("undici": "^6.27.0"), and restores
the dispatcher assertions in the SSRF test so it matches the fork's
byte for byte.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CEguyEgXa9JjCciXCcVemC
Collaborator
|
Implemented the requested review fixes in commit
Verification completed:
|
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.
[Security 1/9] SSRF guardrails for server-side connector fetches
TL;DR
When a user registers an MCP connector, our server fetches the URL they gave us. This PR stops that fetch from being pointed at internal infrastructure (the cloud metadata service, the database, admin panels) by validating the resolved IP address against every private/reserved range and failing closed. It also closes the DNS-rebinding race by pinning the connection to the address we validated.
Risk to user data
Severity: critical. A successful SSRF here is a direct path to total data compromise, not just one user's records:
http://169.254.169.254/…) hands out the server's IAM credentials. With those, an attacker reads the entire object store and database — every tenant's documents, not their own.localhost:5432, private admin panels) are reachable from the server but not from the public internet, so this bypasses the network perimeter entirely.The attacker only needs an ordinary app account and the "add a connector" feature.
Flows affected
validateRemoteMcpUrl)mcp/oauth.ts) — previously these used rawfetchand were unguarded SSRF sinks.Attack precedent
169.254.169.254, retrieved IAM role credentials, and used them to read S3. This exact primitive. (Krebs)sequenceDiagram participant A as Attacker (ordinary user) participant S as Mike backend (inside the VPC) participant M as Metadata service<br/>169.254.169.254 A->>S: Register connector<br/>url = http://169.254.169.254/latest/meta-data/ S->>M: server-side GET (from inside the VPC) M-->>S: IAM credentials S-->>A: response relayed back Note over S,M: With this PR: 169.254.169.254 is link-local →<br/>refused before a socket opensPossible fixes, and what we chose
localhost,metadata)0x7f.0.0.1, IPv6 forms. Rejected.The correct-implementation details that matter:
http://internal.evil.com/looks public but resolves to10.0.0.5.100.64/10), multicast, and the IPv6 forms of all of them — including IPv4 addresses embedded in IPv6 literals (::ffff:10.0.0.1,::7f00:1), the classic filter bypass.privateIp.tsexpands these.302s tohttp://localhost/defeats a naive check — soguardedFetchsetsredirect: "manual".The tests in
client.ssrf.test.tsencode the bypass encodings, not just the happy path.The subtle part: DNS rebinding (a TOCTOU race)
Validating the IP and then fetching the URL is two separate DNS lookups. An attacker who controls DNS for
evil.example(TTL 0) answers the first lookup — the validation — with a harmless public IP, and the second lookup — the real fetch — with127.0.0.1. This is a classic time-of-check-to-time-of-use race: the thing you checked is not the thing you used.sequenceDiagram participant D as Attacker DNS (TTL 0) participant S as Naive server S->>D: resolve evil.example (validation) D-->>S: 93.184.216.34 — public, check passes ✅ S->>D: resolve evil.example (actual fetch) D-->>S: 127.0.0.1 — check already passed ❌ S->>S: connects to 127.0.0.1Fix: resolve once, validate those addresses, and connect to exactly them. This PR does it with a per-request undici
Agentwhoseconnect.lookuphook runs the private-IP guard at the moment the socket opens and hands undici only validated addresses — so the address we validate is the address we connect to. The URL hostname is untouched, so theHostheader and TLS SNI still reflect the real host and certificate verification works normally against legitimate public servers.What's in this PR
backend/src/lib/privateIp.ts— conservative IP classifier (new, shared).backend/src/lib/mcp/client.ts—validateRemoteMcpUrl,guardedFetch, and the connect-time-pinned undici dispatcher.backend/src/lib/mcp/oauth.ts— routes all discovery/registration/refresh throughguardedFetch.undicidependency; tests inclient.ssrf.test.ts(9 tests, incl. embedded-IPv4-in-IPv6 bypasses).Reading
PortSwigger: SSRF · OWASP SSRF Prevention Cheat Sheet