fix(security): block literal private/loopback IPs in SSRF-safe dispatcher#1671
Open
giladresisi wants to merge 1 commit into
Open
fix(security): block literal private/loopback IPs in SSRF-safe dispatcher#1671giladresisi wants to merge 1 commit into
giladresisi wants to merge 1 commit into
Conversation
…cher The ssrfSafeDispatcher enforced its allow-list only inside the undici `connect.lookup` hook. But Node's net.connect only calls `lookup` for hostnames that need DNS resolution — for a literal-IP host it connects directly and never invokes lookup. So any URL with a literal private, loopback, or link-local IP (http://192.168.1.1/, http://127.0.0.1/, http://169.254.169.254/, http://[::1]/, ...) bypassed the guard entirely. This is an SSRF hole affecting every consumer of the dispatcher: the uploadFromUrl agent tool, the public /upload-from-url API, and webhook delivery. Any authenticated API user could make the server issue requests to hosts on its private network (cloud metadata endpoints, internal services, localhost-bound databases) and read back non-2xx / error signal. Fix: move the literal-IP check into a custom `connect` (undici buildConnector), which runs for every connection regardless of DNS. The hostname DNS-pinning path is kept as `pinnedConnector.lookup` so hostnames resolving to private IPs stay blocked too (the TOCTOU protection from GHSA-f7jj-p389-4w45 is preserved). Reproduced (pre-fix), live via the uploadFromUrl agent tool over MCP: http://192.168.1.1/a.jpg -> {"error":"Failed to fetch URL"} i.e. fetch SUCCEEDED — the server connected to the private host and got a response back (the "!response.ok" branch), proving the round-trip happened. Validated (post-fix), same live call: http://192.168.1.1/a.jpg -> {"error":"Failed to upload media from URL: fetch failed"} i.e. the connection is now rejected at the dispatcher before any socket opens (the catch branch). With the companion err.cause change applied it reads "... fetch failed (Blocked IP)". No regression (same live tool): https://picsum.photos/200/300.jpg -> {"id","path"} (public image still uploads) https://httpbin.org/status/404 -> "Failed to fetch URL" (ordinary non-2xx path intact) Also verified with a standalone undici repro: literal 192.168.1.1, 127.0.0.1, 169.254.169.254, [::1] and hostname localhost are all rejected with "Blocked IP", while example.com still connects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Contribution-checker quality warning Heuristics that flagged:
If this is a genuine contribution, please add detail to your PR description and tighten the diff scope before reviewers look at it. |
23 tasks
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.
The vulnerability
ssrfSafeDispatcherenforced its private-IP allow-list only inside the undiciconnect.lookuphook. But Node'snet.connectcallslookuponly for hostnames that need DNS resolution — for a literal-IP host it connects directly and never invokeslookup. So any URL written as a bare private / loopback / link-local IP bypassed the guard completely:http://169.254.169.254/…— cloud metadata endpoint (can expose temporary cloud credentials → account compromise)http://127.0.0.1:<port>/…,http://10.x.x.x/…— internal-only services that assume "only our own network can reach me"http://192.168.1.1/…,http://[::1]/…— local network / loopbackThis is an SSRF hole affecting every consumer of the dispatcher: the
uploadFromUrlagent tool, the public/upload-from-urlAPI (n8n & other external clients), and webhook delivery. Any authenticated API/agent user could make the server issue requests into its own private network and read back signal from the response.The fix
Move the literal-IP check into a custom
connect(built via undici'sbuildConnector), which runs for every connection regardless of DNS. The hostname DNS-pinning path is preserved aspinnedConnector.lookup, so hostnames that resolve to private IPs stay blocked too — the TOCTOU protection from GHSA-f7jj-p389-4w45 is unchanged.Reproduction & validation
Pre-fix, live via the
uploadFromUrlagent tool over MCP:fetch succeeded — the server connected to the private host and got a response (the
!response.okbranch), proving the round-trip happened.Post-fix, same live call:
the connection is now rejected at the dispatcher before any socket opens. (With the companion
err.causePR applied, this reads... fetch failed (Blocked IP).)No regression, same live tool:
Also verified with a standalone undici repro: literal
192.168.1.1,127.0.0.1,169.254.169.254,[::1]and hostnamelocalhostare all rejected withBlocked IP, whileexample.comstill connects.🤖 Generated with Claude Code