fix(security): close SSRF guard bypass via IPv4-mapped IPv6 - #152
Open
ar2rsawseen wants to merge 2 commits into
Open
fix(security): close SSRF guard bypass via IPv4-mapped IPv6#152ar2rsawseen wants to merge 2 commits into
ar2rsawseen wants to merge 2 commits into
Conversation
The HTTP transport's caller-controlled X-Countly-Server-Url header is validated by assertSafeServerHost before the server issues an outbound request (forwarding its configured COUNTLY_AUTH_TOKEN). The previous guard classified IPs with a dotted-quad regex plus a handful of IPv6 string prefixes, so an IPv4-mapped IPv6 address such as `http://[::ffff:127.0.0.1]` (which Node normalizes to `[::ffff:7f00:1]` and the OS routes to IPv4 loopback) matched neither branch and slipped through — a full-read SSRF against loopback/private/metadata targets. Replace the hand-rolled classification with ipaddr.js range() detection, mirroring countly-server's api/utils/ssrf-protection.js: only globally routable `unicast` addresses are allowed; IPv4-mapped IPv6 is unwrapped to its embedded IPv4 and re-classified. Also: - block the .internal TLD and known cloud-metadata / k8s hostnames - strip IPv6 brackets before classifying - reject URLs with embedded credentials (user:pass@host) Adds regression tests covering ::ffff:127.0.0.1, ::ffff:169.254.169.254, the hex-collapsed and bracketed forms, IPv4-mapped RFC1918, and embedded credentials. Note: TEST-NET (RFC5737) documentation ranges are now correctly rejected as `reserved`, matching countly-server behavior. Co-Authored-By: Claude <noreply@anthropic.com>
…r URLs The syntactic host check only classifies IP literals; a caller-supplied hostname is never resolved, so a name whose A/AAAA record points at a private/loopback/metadata IP passed straight through, and DNS-rebinding (resolve-public-then-flip) defeated any parse-time-only check. Port countly-server's safeLookup: a dns.lookup-compatible function that re-classifies the resolved address (via the shared ipaddr.js range check, IPv4-mapped unwrapped) and fails the lookup with ESSRFBLOCKED for any non-public target. Wire it — plus maxRedirects: 0 — into the per-request axios client through http/https Agents, but ONLY for the caller-controlled path: RequestState gains serverUrlFromCaller, set by the HTTP middleware when the X-Countly-Server-Url header / URL param is present. The operator's trusted COUNTLY_SERVER_URL (often a private-IP on-prem host) skips the guard and keeps working. This closes both remaining DNS gaps at connect time: (1) a hostname that simply resolves to an internal IP, and (2) DNS-rebinding TOCTOU. Adds safeLookup unit tests (public pass-through, loopback/metadata/ IPv4-mapped block, options.all form, callback-as-second-arg form). Co-Authored-By: Claude <noreply@anthropic.com>
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.
Summary
Fixes an unauthenticated full-read SSRF in the Streamable HTTP transport, reported by
roopd3v(r00p) against v1.4.0, and hardens the surrounding DNS handling to match countly-server's SSRF protection.The caller-controlled
X-Countly-Server-Urlheader is validated byassertSafeServerHostbefore the server issues an outbound request forwarding its configuredCOUNTLY_AUTH_TOKEN. The old guard classified IPs with a dotted-quad regex plus a few IPv6 string prefixes, so an IPv4-mapped IPv6 address likehttp://[::ffff:127.0.0.1]— which Node normalizes to[::ffff:7f00:1]and the OS routes to IPv4 loopback — matched neither branch and slipped through.Reproduction (before fix)
With
COUNTLY_MAX_BODY_BYTES=0, a single unauthenticatedPOST /mcpwithX-Countly-Server-Url: http://[::ffff:127.0.0.1]:<port>caused the server to fetch loopback and return the internal response body to the caller, with the server's configured token attached:(Under the default positive
COUNTLY_MAX_BODY_BYTES, the body-size middleware consumes the stream first, so the SSRF is only reachable in that non-default config — but the guard flaw itself is real and version-independent.)Commit 1 — close the IPv4-mapped IPv6 guard bypass
Replaces the hand-rolled IP classification with
ipaddr.jsrange()detection, mirroring countly-server'sapi/utils/ssrf-protection.js:unicastaddresses are allowed; every other range (loopback, private, link-local, unique-local, CGNAT, multicast, reserved, unspecified, NAT64, …) is blocked..internalTLD and known cloud-metadata / k8s hostnames.user:pass@host).Commit 2 — connect-time DNS validation (closes the DNS gaps)
The syntactic check above only classifies IP literals. A caller-supplied hostname was never resolved, leaving two gaps:
ssrf.attacker.com → A 169.254.169.254passed the guard outright.Ports countly-server's
safeLookup: adns.lookup-compatible function that re-classifies the resolved address (sharedipaddr.jscheck, IPv4-mapped unwrapped) and fails withESSRFBLOCKEDfor any non-public target. Wired — together withmaxRedirects: 0— into the per-request axios client viahttp/httpsAgents, so the socket only ever connects to a public unicast IP and a 30x can't bounce to an internal target.Scoped to the caller-controlled path only.
RequestStategainsserverUrlFromCaller, set by the HTTP middleware when the header/param is present. The operator's trustedCOUNTLY_SERVER_URL(frequently a private-IP on-prem box) skips the guard and keeps working — verified by test.Verification
::ffff:127.0.0.1header → HTTP 400, 0 canary hits.lvh.me/nip.iohostname resolving to127.0.0.1→ blocked at connect (Blocked SSRF target … resolved to non-public IP), 0 canary hits.COUNTLY_SERVER_URL=http://127.0.0.1:<port>with no caller header → still reaches the target (on-prem private IP unaffected).Behavior note
TEST-NET (RFC5737) documentation ranges (e.g.
203.0.113.0/24) are now correctly rejected asreserved, matching countly-server. The on-prem test IP was updated to a routable public address accordingly.Follow-up (not in this PR)
Parse error: Invalid JSON. Separate bug, flagged for triage.🤖 Generated with Claude Code