fix: eliminate DNS rebinding TOCTOU and redirect bypass in SSRF protection (CWE-918) - #45
Closed
sebastionoss wants to merge 1 commit into
Closed
Conversation
Resolve DNS once via resolveAndPin(), validate the IP is not private, then rewrite the URL to use the validated IP directly with appropriate Host header and TLS SNI. This prevents DNS rebinding attacks where a second DNS resolution (inside fetch) could return a different IP. Also switches to manual redirect handling so every redirect hop is validated (protocol, hostname, and resolved IP) before the request is made, preventing SSRF via open redirects. CWE-918
Author
|
Closing this as inactive — no maintainer response after 30 days. The security finding and fix remain valid. If this is still relevant, I'm happy to reopen, rebase, or re-submit against a different branch. Just drop a comment. |
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.
Vulnerability Summary
CWE-918: Server-Side Request Forgery (SSRF)
Severity: High
Affected file:
src/Fetcher.ts—_fetch()method andvalidateResolvedIp()helperData Flow
The URL parameter originates from MCP tool calls, which can be influenced by user input or prompt injection. There is no authentication layer, no allowlist, and no network-level mitigation between the tool call and the outbound request.
Two Distinct Attack Vectors
1. DNS Rebinding TOCTOU (Full Read SSRF)
validateResolvedIp()performs DNS resolution and validates the IP, butfetch()resolves DNS again internally. An attacker controlling a DNS server with TTL=0 can return a public IP for validation and a private IP (e.g.,169.254.169.254for cloud metadata) for the actual fetch. This is a classic Time-of-Check-to-Time-of-Use race condition.2. Redirect Bypass (Blind SSRF)
The original code follows redirects automatically (
fetch()default behavior) and only validatesresponse.urlafter the request has completed. A redirect to an internal IP (e.g.,http://127.0.0.1:8080/admin/delete-all) executes the request and potentially triggers side effects before the post-check throws an error.Exploit Sketch
DNS Rebinding:
Redirect Bypass:
Preconditions
rebind.networkexist)Fix Description
Approach: IP Pinning + Manual Redirect Following
The fix replaces the vulnerable validate-then-fetch pattern with a resolve-once-use-once approach:
resolveAndPin(url)— New method that resolves DNS once, validates the IP is not private, then rewrites the URL to use the resolved IP directly (e.g.,http://93.184.216.34:8080/path). Returns both the pinned URL and the original hostname for Host header / TLS SNI.Manual redirect handling —
fetch()is called withredirect: "manual". Each redirect hop is validated and pinned before following, in a bounded loop (max 20 redirects). This ensures no request to a private IP is ever made, even via redirect chains.Host header and TLS SNI preservation — Since the URL hostname is replaced with an IP, the original
Hostheader is explicitly set, and TLSserverNameis set for HTTPS to ensure proper certificate validation and virtual hosting.Why this approach?
IP pinning is the OWASP-recommended defense against DNS rebinding TOCTOU in SSRF protections. Manual redirect following with per-hop validation is the standard defense against redirect-based bypasses. Both are well-established patterns.
Changes
src/Fetcher.tsvalidateResolvedIp()withresolveAndPin(). Rewrote_fetch()to use manual redirect loop with per-hop validation. AddedMAX_REDIRECTSconstant.src/Fetcher.test.tsResponseobjects with status 302 + Location header instead ofresponse.url).src/Fetcher.ssrf.test.tsTest Results
All 107 tests pass (101 existing + 6 new SSRF-specific):
Existing tests for HTML fetching, JSON parsing, Readability, YouTube transcripts, URL validation, and content-length limits continue to pass.
Disprove Analysis
To ensure this is a genuine finding, we systematically attempted to disprove it across 8 dimensions:
Fetcher.tsorindex.ts. MCP protocol relies on host app trust. URL comes directly from AI tool calls. No mitigation.npx mcp-fetch-serveron user machine or VM. No reverse proxy._fetch()called by 7 public MCP tools. URL param comes from AI tool call (Zod-validated asz.string().url()— schema only, no content validation). Attacker-controlled data reaches vulnerable function.validateUrl()blockslocalhost, private IPs, non-HTTP protocols.validateResolvedIp()does DNS lookup + private IP check — but vulnerable to TOCTOU (two DNS lookups). Redirect check fires after request completes.chore: bump to v1.1.2). DNS rebinding fix not previously applied.Fix Adequacy Note
The
proxyparameter (...(proxy ? { proxy } : {})) provides a separate SSRF bypass path that is not addressed by this fix — it is the subject of a separate report (PR #44). These are distinct vulnerability classes: even with the proxy fix applied, DNS rebinding and redirect bypass would remain exploitable without this fix. Conversely, this fix closes attack vectors that cannot be reached via proxy bypass.Prior Art
DNS rebinding TOCTOU is well-documented:
Verdict: CONFIRMED_VALID | Confidence: High
Diff Summary
Thank you for maintaining fetch-mcp! Happy to discuss any aspect of this fix or adjust the approach if you have preferences.