fix(web-fetch): resolve DNS before SSRF guard to block hostname-to-private-IP bypass#27744
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical SSRF vulnerability where attackers could bypass security guards by using wildcard DNS services or specific hostname patterns that resolve to private network addresses. By shifting from synchronous IP validation to an asynchronous approach that resolves DNS before checking against private IP ranges, the system now correctly identifies and blocks these malicious requests. The changes include updating core validation logic and expanding the static blocklist to cover additional loopback and unspecified address formats. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
📊 PR Size: size/L
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
cdf74eb to
9e8c502
Compare
There was a problem hiding this comment.
Code Review
This pull request updates the WebFetchTool to use asynchronous DNS resolution (isPrivateIpAsync) to prevent SSRF bypasses via hostname-to-private-IP mappings (such as nip.io), and updates the unit tests accordingly. The review feedback identifies a critical DNS rebinding vulnerability because the subsequent HTTP request still uses the original hostname instead of the validated IP address. Additionally, the feedback suggests synchronously blocking the IPv6 unspecified address [::] and adding corresponding test coverage.
| // Resolve DNS to catch hostname-to-private-IP bypasses such as | ||
| // 127.0.0.1.nip.io, 169.254.169.254.nip.io, or attacker-controlled DNS | ||
| try { | ||
| return await isPrivateIpAsync(urlStr); | ||
| } catch { |
There was a problem hiding this comment.
DNS Rebinding Vulnerability leading to SSRF Bypass
Vulnerability Description:
The SSRF protection implemented in isBlockedHost resolves the URL's hostname to its IP addresses using isPrivateIpAsync (which uses dns.promises.lookup) and checks if any of the resolved IPs are private. However, the subsequent HTTP request in fetchWithTimeout is made using the original URL string (which contains the hostname, not the resolved IP address).
This introduces a classic DNS Rebinding vulnerability. An attacker can configure a malicious DNS server to return a public IP address on the first resolution (bypassing the isBlockedHost check) and then return a private IP address (such as 127.0.0.1 or 169.254.169.254) on the second resolution (when fetch actually connects to the host). This allows the attacker to bypass the SSRF guard completely and access internal resources or cloud metadata endpoints.
Remediation:
To prevent DNS rebinding, the application should either:
- Pin the resolved IP address: Resolve the hostname to an IP address, validate it, and then perform the HTTP request directly to that IP address (while setting the
Hostheader to the original hostname). - Use a custom undici Agent/Dispatcher: Configure a custom undici
AgentorDispatcherwith a customconnectorlookupfunction that validates the resolved IP address at connection time, ensuring that the IP address actually connected to is the same one that was validated.
There was a problem hiding this comment.
Thanks for the review. I've already implemented the recommended fix: IP pinning. The hostname is resolved once, validated, and then the fetch is performed directly to the resolved IP address with the original Host header set. This prevents DNS rebinding. The changes have been pushed (commit fe2a8ef). Please re-review.
| if (isLoopbackHost(hostname) || hostname === '0.0.0.0') { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The IPv6 unspecified address [::] can behave similarly to loopback/unspecified IPv4 0.0.0.0 on many platforms, potentially routing to localhost. While isPrivateIpAsync will eventually block it, doing so requires a DNS lookup that is guaranteed to fail (due to square brackets in the hostname passed to dns.lookup) or throw an error. Explicitly blocking [::] synchronously alongside 0.0.0.0 is more robust, avoids unnecessary DNS lookup errors, and ensures consistent protection.
| if (isLoopbackHost(hostname) || hostname === '0.0.0.0') { | |
| return true; | |
| } | |
| if (isLoopbackHost(hostname) || hostname === '0.0.0.0' || hostname === '[::]') { | |
| return true; | |
| } |
There was a problem hiding this comment.
Done. Added hostname === '[::]' to the synchronous blocklist. The change is included in the latest push. Thanks.
| label: '0.0.0.0 (unspecified literal)', | ||
| url: 'http://0.0.0.0', | ||
| dnsPrivate: false, | ||
| }, | ||
| { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Added test case for http://[::] as suggested. It's now covered in the SSRF guard test suite. Please re-check.
ce44fea to
fe2a8ef
Compare
|
@google-gemini/gemini-cli-prompt-approvers Could someone please approve the eval-gate workflow for this PR? The SSRF fix is complete (IP pinning, block [::], tests added) and needs to run CI. Thanks! |
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. This PR will be closed in 7 days if it remains without that designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
312351b to
17a70c7
Compare
|
Rebased onto latest main ,now cleanly mergeable. This PR fixes an SSRF vulnerability reported to Google Cloud VRP (Issue 520981635); patch is complete with IP pinning (DNS rebinding), [::] blocking, and full test coverage. Since this is a security fix rather than a feature, could a maintainer consider it for review before the auto-close window? |
|
This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
Summary
isBlockedHost()called the synchronousisPrivateIp()which usesipaddr.isValid()to test whether the URL's hostname is itself an IP address in a private range. Any non-IP-literal hostname — including wildcard-DNS services like127.0.0.1.nip.ioor169.254.169.254.nip.io— passesipaddr.isValid()asfalse, soisAddressPrivate()returnsfalseand the guard silently allows the request.isBlockedHost()async and replace the synchronous call withisPrivateIpAsync()(already present inutils/fetch.tsbut unused here), which resolves all DNS addresses and validates each againstisAddressPrivate(). Fail-closed on DNS errors. Expand the explicit literal blocklist to also cover::1/[::1]and0.0.0.0.Bypass vectors (pre-fix)
127.0.0.1.nip.ioipaddr.isValid()false → allowed169.254.169.254.nip.iolocaltest.me10.x,172.16.x,192.168.x, etc.Impact
An LLM agent processing attacker-controlled content (indirect prompt injection) can be instructed to call
web_fetchwith a crafted hostname. Before this fix:http://127.0.0.1.nip.io:<port>/reaches any service bound to loopback (e.g., local dev servers, internal admin APIs).http://169.254.169.254.nip.io/latest/meta-data/fetches AWS/GCP/Azure instance metadata including IAM credentials when running in CI or cloud environments.web_fetchinvocation targeting an internal endpoint, exfiltrating secrets back through the LLM response.Fix approach
isBlockedHost()is nowasync.localhost,127.0.0.1,::1,[::1],0.0.0.0) synchronously, the method callsisPrivateIpAsync(), which resolves the hostname vianode:dns/promiseslookup({ all: true })and checks every returned address withisAddressPrivate().return true(deny by default).executeFallbackForUrl,filterAndValidateUrls,executeExperimental) and theexecutedispatcher are updated toawaitthe guard.Tests added
New
describe('SSRF guard — DNS hostname-to-private-IP bypass')suite covering:localhost→ blocked (explicit literal)127.0.0.1→ blocked (explicit literal)::1→ blocked (explicit literal, new)0.0.0.0→ blocked (explicit literal, new)127.0.0.1.nip.io→ blocked (DNS mock returns private IP)169.254.169.254.nip.io→ blocked (DNS mock returns link-local)https://google.com→ allowedfilterAndValidateUrlsalso covered.Security report reference
This vulnerability was reported to Google Cloud VRP on 2026-06-07 (Issue #520981635). A working proof-of-concept was submitted demonstrating end-to-end exfiltration of loopback-only content via
127.0.0.1.nip.ioand the cloud metadata endpoint via169.254.169.254.nip.io, both bypassing the existingisBlockedHost()guard. This PR is submitted as a Patch Rewards contribution to strengthen the report and demonstrate the remediation path.