fix(security): SSRF guard for the LLM fallback fetch (H3) - #15
Conversation
PR-C of the security-hardening plan. - New always-compiled core::net_guard: guard_and_resolve() allows only http/https, resolves the host, and refuses if ANY resolved IP is non-public (loopback, RFC1918, link-local incl. 169.254.169.254 metadata, CGNAT, benchmarking, reserved; IPv6 ULA/link-local; IPv4-mapped/compatible recursion; 6to4/Teredo/NAT64 transition prefixes). Returns the validated IPs to pin. - browser.rs fetch_html: pins the validated IPs (.resolve, anti-rebinding), disables redirects, disables system proxies (.no_proxy, so a proxy can't re-resolve the host), 30s timeout, and a fail-closed host allowlist (RELAIS_FALLBACK_ALLOW). Imported cookies are sent only when the host matches the cookie's stored domain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cad1e379b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .no_proxy() | ||
| .timeout(Duration::from_secs(30)); | ||
| for addr in &target.addrs { | ||
| builder = builder.resolve(&target.host, *addr); |
There was a problem hiding this comment.
When an allowlisted host has more than one validated A/AAAA record, ClientBuilder::resolve is a single-address override for a domain, so each iteration replaces the previous override and the client only tries the last address. In dual-stack or multi-A deployments where that last address is unreachable, the fallback fetch fails even though another validated address was available; pass the whole target.addrs slice via resolve_to_addrs instead.
Useful? React with 👍 / 👎.
| (s[0] & 0xfe00) == 0xfc00 // fc00::/7 unique-local | ||
| || (s[0] & 0xffc0) == 0xfe80 // fe80::/10 link-local |
There was a problem hiding this comment.
When an allowlisted hostname resolves to a deprecated IPv6 site-local address (fec0::/10), this predicate falls through because only ULA and link-local ranges are checked. Those addresses are non-public, so the new guard can allow DNS rebinding to internal IPv6 services on networks that still route site-local space; add an explicit fec0::/10 block.
Useful? React with 👍 / 👎.
1 participant
PR-C of the security-hardening plan. Finding H3: the LLM fallback fetches a caller-supplied URL and attaches imported cookies with no host/IP validation (SSRF + cookie exfiltration).
core::net_guard(new, always compiled)guard_and_resolve(url): http/https only → resolve host → reject if any resolved IP is non-public.is_blocked_ipcovers v4 loopback/RFC1918/link-local (incl.169.254.169.254metadata)/CGNAT/benchmarking/reserved/broadcast/unspecified/multicast/documentation, and v6 loopback/unspecified/multicast/ULA(fc00::/7)/link-local(fe80::/10), plus IPv4-mapped & IPv4-compatible recursion and the 6to4 / Teredo / NAT64 transition prefixes (so64:ff9b::169.254.169.254etc. can't smuggle a blocked v4). Returns the validated IPs to pin.host_matches_cookie_domain(host, domain)for cookie scoping.browser.rsguarded fetch.resolve, anti-DNS-rebinding), disables redirects (Policy::none()), disables system proxies (.no_proxy()— a proxy would otherwise re-resolve the host and bypass the pin), 30s timeout.RELAIS_FALLBACK_ALLOW=host1,host2); unset ⇒ no host allowed.CookieScope { domain, cookies }).The fallback adapter is not registered by default, so this is fail-closed hardening for when it is enabled.
Tests
is_blocked_ipacross v4 + v6 transition/mapped/compatible forms; public addresses allowed; loopback URL blocked offline; bad scheme rejected; cookie-domain scoping (incl.example.com.evil.comnot matchingexample.com).cargo test --workspacegreen; core clippy clean.Codex-reviewed; the two flagged HIGH bypasses (default system-proxy resolution, IPv6 transition/NAT64 forms) are fixed.
🤖 Generated with Claude Code