Skip to content

fix(core): classify IPv6 literals by parsed bytes in the private-range SSRF gate - #3502

Merged
Astro-Han merged 4 commits into
apache:mainfrom
GabrielDrapor:fix/private-range-ipv6-spellings
Aug 23, 2026
Merged

fix(core): classify IPv6 literals by parsed bytes in the private-range SSRF gate#3502
Astro-Han merged 4 commits into
apache:mainfrom
GabrielDrapor:fix/private-range-ipv6-spellings

Conversation

@GabrielDrapor

Copy link
Copy Markdown
Contributor

Follow-up to @Astro-Han's review note on #2653 about the isLoopbackHost/isPrivateRangeHost IPv6 blind spot, now that the code lives on main via #2920's squash.

The gap

isPrivateRangeHost matched hostname spellings — and WHATWG URL parsing canonicalizes them before the gate ever sees one: [::ffff:127.0.0.1] arrives as [::ffff:7f00:1], which matched nothing, so every IPv4-mapped spelling of a loopback or private destination passed the remote-provenance https check (fail open, exactly the direction the note called out). The 'fe8' prefix check also covered only a quarter of fe80::/10 — fe9x/feax/febx link-local spellings slipped through.

The fix — with the opposite-defaults treatment from the review

The literal now parses to its 16 bytes (a minimal RFC 4291 text parser, no resolver) and classifies structurally:

  • IPv4-mapped (::ffff/96) and NAT64 (64:ff9b::/96) embeddings classify by their embedded IPv4 — including 127/8, which lands on the PRIVATE side of the gate on purpose;
  • isLoopbackHost deliberately does not learn the mapped spellings: its unknown-spelling default stays "not loopback" so cleartext stays refused (fail closed), while this gate's default stays "private" so the request is blocked (also fail closed) — the two defaults point in opposite directions, as the note prescribed;
  • an unparseable bracketed literal is treated as private;
  • fc00::/7 and the full fe80::/10 are matched on bytes;
  • global IPv6 stays reachable ([2606:4700::1], mapped 8.8.8.8) — real OAuth endpoints on IPv6 keep working.

(The #2920 loopback listener is unaffected, as the note observed — it pins Host to the exact 127.0.0.1:<port> literal.)

Regressions

  • packages/core: unit coverage across spellings of one address (raw, uppercase, uncompressed, canonical), the full fe80::/10 range, NAT64, [::1]/loopback asymmetry, global reachability, and garbage-in-brackets failing closed.
  • packages/mcp transport-security: end-to-end — mapped private/loopback https refused under remote provenance; public IPv6 allowed; cleartext to the mapped loopback still refused by the strict loopback predicate.

Suites: core 608, mcp 171 — green; biome + ASF header audit clean.

Co-Authored-By: Claude noreply@anthropic.com

https://claude.ai/code/session_01TMwYxgNEbz2RFmuK6AXGcj

…e SSRF gate

isPrivateRangeHost matched hostname spellings, and WHATWG URL parsing
canonicalizes them: '[::ffff:127.0.0.1]' reaches the gate as
'[::ffff:7f00:1]', which matched nothing — so every IPv4-mapped spelling
of a loopback or private destination sailed through the remote-provenance
https check (fail open). The 'fe8' prefix check also covered only a
quarter of fe80::/10, missing fe9x/feax/febx link-local spellings.

The literal now parses to its 16 bytes (minimal RFC 4291 text parser) and
classifies structurally: IPv4-mapped (::ffff/96) and NAT64 (64:ff9b::/96)
embeddings classify by their embedded IPv4 — including 127/8, which lands
on the PRIVATE side of the gate on purpose. isLoopbackHost deliberately
does NOT learn the mapped spellings: its unknown-spelling default must
stay 'not loopback' so cleartext is refused (fail closed), while this
gate's must stay 'private' so the request is blocked (also fail closed) —
the two defaults point in opposite directions, per review guidance on
apache#2653. An unparseable bracketed literal is treated as private.

Regressions: core unit coverage across spellings, the full fe80::/10
range, NAT64, global-IPv6 reachability, and garbage-in-brackets; plus
transport-security end-to-end cases asserting mapped private/loopback
https is refused under remote provenance while cleartext to the mapped
loopback stays refused by the strict loopback predicate.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TMwYxgNEbz2RFmuK6AXGcj

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — moving this from spelling to parsed bytes is the right call, and the 16-byte normalization handles the cases that string matching kept getting wrong: compressed zeros, ::ffff: mapped, NAT64, the full fe80::/10 rather than just the fe8 prefix, and garbage input. We confirmed the old string path is gone rather than left as a fallback.

Reviewed at exact head 109cbfca25883b6b079b1eea7d8a0158eb592e4b against base 56d03b4eab1e5b1980ca93775a9ae2e926cb61c9. One P2, inline. No checks have run on this head yet.

The gap is one prefix: classifyIpv6 special-cases ::ffff:/96 and 64:ff9b::/96 but not IPv4-compatible ::/96, so an embedded IPv4 written that way falls through to global. Details and a reproduction are inline.

Two things we checked and are not reporting: the parser strips %zone before classification so fe80::1%eth0 still lands on private, and WHATWG URL currently rejects URI zone-ids anyway; and a spelled-out loopback like ::0:1 normalizes to [::1] and is caught by the existing loopback check.


This review was AI-assisted. Findings were verified against the exact head listed above; any mistakes are ours to correct — please push back where we got it wrong.

Comment thread packages/core/src/mcp.ts
return bytes;
}

function classifyIpv6(bytes: Uint8Array): 'loopback' | 'private' | 'global' {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] IPv4-compatible ::/96 is not classified by its embedded IPv4.

classifyIpv6 recognizes two ways of embedding an IPv4 address — ::ffff:/96 and 64:ff9b::/96 — and hands both to isPrivateIpv4. It does not recognize the third, ::/96, so those addresses reach the final return 'global'.

Reproduction, using the URL parser this gate sits behind:

new URL('https://[::192.168.1.1]/').hostname  // '[::c0a8:101]'
new URL('https://[::127.0.0.1]/').hostname    // '[::7f00:1]'

Trace [::7f00:1] through the function: bytes 0–11 are zero so mapped is false (byte 10 is not 0xff) and nat64 is false (byte 1 is not 0x64); the loopback check requires bytes 0–14 to be zero and byte 12 is 0x7f; fc00::/7 and fe80::/10 both test byte 0, which is zero. Result: global. isPrivateRangeHost('[::7f00:1]') returns false, so the remote-HTTPS branch in transport-security.ts:59-75 does not reject it, and a remote server's metadata or a redirect can still hand back a target spelled this way.

On severity. We tried to connect to ::127.0.0.1 on macOS and it times out rather than reaching the loopback listener, so on a current stack this is unlikely to be an exploitable path to a live service — IPv4-compatible addresses are deprecated by RFC 4291 and most stacks no longer translate them. That is why this is a P2 and not a P1. But it should still be closed, for a reason that does not depend on reachability: this PR's whole premise is that classification happens on parsed bytes rather than on how an address is written, and ::/96 is the one byte-level IPv4 embedding the classifier does not know about. Leaving it means the gate's correctness rests on an assumption about the other end's network stack, which is exactly the kind of assumption this change set out to remove.

Smallest fix: treat ::/96 alongside the other two — bytes 0–11 all zero, excluding :: and ::1 which the surrounding checks already own — and reuse the same isPrivateIpv4 / 127. logic. Worth a regression case that comes in through the remote-provenance path rather than only calling the classifier directly, since that is where a real one would arrive.

Review follow-up on apache#3502: classifyIpv6 knew two byte-level IPv4
embeddings (::ffff/96, 64:ff9b::/96) but not the deprecated
IPv4-compatible ::/96 (RFC 4291 §2.5.5.1) — '[::192.168.1.1]'
canonicalizes to '[::c0a8:101]' and fell through to 'global'. The
classifier's premise is bytes over spellings, and whether the other end's
stack still translates the deprecated form must not be what the gate's
correctness rests on.

::/96 now classifies by its embedded IPv4 like the other two. '::1' is
returned by the loopback check first; the all-zero '::' (embedded
0.0.0.0) is treated as private — connecting to the unspecified address
reaches the LOCAL machine on common stacks, so it fails closed. Public
v4-compatible embeddings (e.g. ::8.8.8.8) stay global.

Regressions: core unit cases across the URL round-trip and raw canonical
forms, plus transport-security cases arriving through the
remote-provenance path.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TMwYxgNEbz2RFmuK6AXGcj
@GabrielDrapor

Copy link
Copy Markdown
Contributor Author

Fixed at head ba4f36d74::/96 now classifies alongside the other two embeddings: bytes 0–11 all zero routes to the same embedded-IPv4 logic, with ::1 returned by the loopback check first (moved above the embedding checks so the compat path can never mis-claim it).

One deliberate extension beyond the smallest fix, flagged for push-back: the all-zero :: (embedded 0.0.0.0) is classified private rather than excluded — connecting to the unspecified address reaches the local machine on common stacks (Linux treats it as connect-to-loopback), and nothing else owned it: isLoopbackHost doesn't match [::], so under the previous exclusion https://[::]:8443/ would have passed the remote-provenance gate. Public v4-compatible embeddings (::8.8.8.8) stay global.

Regressions added in both places you suggested: core unit cases across the URL round-trip (https://[::192.168.1.1]/[::c0a8:101]) and raw canonical forms, plus transport-security cases arriving through the remote-provenance path ([::192.168.1.1], [::127.0.0.1]:8443, [::]). Suites: core 609, mcp 171 — green.

…SSRF gate

Independent adversarial review (Codex) against the previous head found
three gaps, all fixed here:

- The mapped unspecified address ([::ffff:0:0]) classified as global;
  connecting to ::ffff:0.0.0.0 verifiably reaches a 127.0.0.1-bound
  listener. The embedded-0.0.0.0 → private rule now applies under EVERY
  embedding, not only IPv4-compatible.
- RFC 8215's local-use NAT64 space (64:ff9b:1::/48) and RFC 2765 SIIT
  (::ffff:0:0/96) were unclassified. The /48 is reserved for in-network
  translation and deployments carve arbitrary RFC 6052 prefix lengths out
  of it, so the whole prefix fails closed; SIIT classifies by its
  embedded IPv4 like the other exact /96 prefixes.
- The parser accepted dotted IPv4 outside the low-order 32 bits
  ('[192.168.1.1::]' parsed shifted instead of failing) and an empty
  zone id — both violated RFC 4291 and the documented fail-closed
  contract. WHATWG URL parsing rejects these today, so the transport path
  was covered, but the exported classifier now honors its own contract.

Regressions in both suites, including transport-security cases through
the remote-provenance path.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TMwYxgNEbz2RFmuK6AXGcj
@GabrielDrapor

Copy link
Copy Markdown
Contributor Author

Follow-up at head f0707b1e8, from an independent adversarial pass (Codex) I ran against the previous head — three findings, all fixed:

  • [P1] [::ffff:0:0] classified global. A runtime probe confirmed connecting to ::ffff:0.0.0.0 reaches a 127.0.0.1-bound listener. The embedded-0.0.0.0 → private rule now applies under every embedding, not only IPv4-compatible.
  • [P1] Translation ranges unclassified. RFC 8215's local-use NAT64 space 64:ff9b:1::/48 now fails closed as a whole (it is reserved for in-network translation, and deployments carve arbitrary RFC 6052 prefix lengths out of it, so the embedded IPv4's position is not recoverable); RFC 2765 SIIT ::ffff:0:0/96 classifies by its embedded IPv4 like the other exact /96 prefixes.
  • [P2] Parser over-acceptance. Dotted IPv4 outside the low-order 32 bits ([192.168.1.1::]) and an empty zone id now fail parsing — and therefore fail closed as private — instead of parsing shifted. WHATWG URL rejects these spellings today, so the transport path was already covered; the exported classifier now honors its own documented contract regardless of caller.

Regressions for all three in both suites, including transport-security cases through the remote-provenance path ([::ffff:0:0]:8443, the /48 encoding of 192.168.1.1, SIIT). core 611, mcp 171 — green.

@Astro-Han

Copy link
Copy Markdown
Contributor

Heads up on CI: the workflow run for this PR was sitting in action_required (fork PRs need a maintainer to approve the run), so it had never actually executed. I approved it — it has now run on f0707b1e89338ca3bc5f4999cd04031f001c25bf and came back red.

Failing step: Check formatting (Biome), one file — packages/core/src/mcp.ts, two sites:

  • around L145, the out.push(...) for the dotted-quad octets wants to be split across lines
  • around L187, the six-way bytes[0] === 0 && ... NAT64 64:ff9b::/96 check wants to be broken up

npm run format (or biome format --write) should settle both — it's purely layout, no logic involved.

Worth flagging explicitly: Check formatting runs early in the job, so everything downstream was reported as skipped, not passed — Build, Typecheck, Run affected standard workspace tests, Run Runtime Host tests, and the rest. The two new test files in this PR (mcp-host-classification.test.ts and the transport-security.test.ts additions) have therefore never run in CI. Red-at-formatting is not a signal about the classification logic in either direction.

On the review: this is a private-range SSRF gate, so we're giving it full adversarial treatment (two independent lines plus a mechanical fact check) regardless of diff size — a wrong private/public verdict here fails silently, with no error and no log, which is exactly the failure mode worth spending review budget on. Conclusions will follow in a separate comment; nothing blocking from us yet beyond the formatting fix.

@Astro-Han

Copy link
Copy Markdown
Contributor

Second review pass on f0707b1e89338ca3bc5f4999cd04031f001c25bf. We ran two independent lines on this one plus a hands-on reachability check, because a private-range gate fails silently — a wrong verdict produces no error and no log, just a request that should have been blocked and wasn't.

The parsing rewrite itself holds up. Classifying by the 16 parsed bytes instead of by spelling is the right move, and it is a genuine simplification: startsWith('fc' | 'fd' | 'fe8') goes away and there is now one authority on what an address is, rather than a growing list of textual forms. Mapped, NAT64, compat and SIIT embeddings all classify by their embedded IPv4; an unparseable bracketed literal falls to private; isLoopbackHost deliberately stays strict-by-spelling so the two defaults fail closed in opposite directions. We checked fe9a:: / febf:: (now private, previously missed by the fe8 prefix), [::ffff:7f00:1] (the form WHATWG canonicalizes [::ffff:127.0.0.1] into), ::/96, SIIT and 64:ff9b:1::/48 — all now behave.

One finding.

[P1] The bare IPv4 unspecified address 0.0.0.0 is still classified as public

isPrivateRangeHost sends IPv4 literals down a separate path from the IPv6 one:

  • packages/core/src/mcp.ts:100-102 — the v4 branch calls isPrivateIpv4(Number(v4[1]), Number(v4[2])), passing only the first two octets.

  • packages/core/src/mcp.ts:110-116isPrivateIpv4 has no case for a === 0, so isPrivateIpv4(0, 0) === false.

  • packages/core/src/mcp.ts:208-212 — the embedded path already handles exactly this address, with this rationale written into the comment:

    The unspecified embedded address (0.0.0.0) under ANY embedding: connecting to it reaches the LOCAL machine on common stacks (::ffff:0:0 verifiably reaches a 127.0.0.1-bound listener) — private, fail closed.

That reasoning is correct, and it is already applied to every IPv6 embedding of the address. It just never reaches the bare v4 literal, which does not pass through classifyIpv6 at all.

Nothing downstream catches it: isLoopbackHost only recognizes localhost, [::1] and 127.x.x.x, and the https branch in transport-security.ts:59-75 allows anything that is neither loopback nor private.

Reproduction (Node 22, listener bound to 127.0.0.1, placeholder port):

isPrivateRangeHost('0.0.0.0')            -> false
isLoopbackHost('0.0.0.0')                -> false
new URL('https://0/').hostname           -> '0.0.0.0'      // also 'https://0x0/'
fetch('http://0.0.0.0:<port>/')          -> 200 local-ok   // reaches the 127.0.0.1 listener
fetch('http://0:<port>/')                -> 200 local-ok

For contrast, on this same head https://[::]/ and https://[::ffff:0:0]/ are both correctly refused. Same machine, same reachability, opposite verdicts depending on how the address is spelled — which is the exact class of inconsistency this PR set out to remove.

The https://0/ form matters: WHATWG canonicalizes it to 0.0.0.0, so a redirect target or an OAuth metadata document only needs to say https://0/ to get through the gate.

Suggested fix: have isPrivateIpv4 see all four octets and treat only 0.0.0.0 as private. We'd suggest not pulling in all of 0.0.0.0/8 — we checked 0.0.0.1 and it does not reach a local listener, so widening the private set there would cost coverage without buying anything. Worth adding https://0.0.0.0/ and https://0/ to the transport-security tests.

CI

Separately from the review: the run on this head is red at Check formatting (the two mcp.ts layout sites noted in our earlier comment). Everything after that step is skipped, so the two new test files in this PR have still never executed in CI.

No P0/P2/P3 findings. Not approving yet — the P1 above, and no terminal-green required check on this head.

Comment thread packages/core/src/mcp.ts Outdated
Comment on lines +110 to +116
function isPrivateIpv4(a: number, b: number): boolean {
if (a === 10) return true;
if (a === 172 && b >= 16 && b <= 31) return true;
if (a === 192 && b === 168) return true;
if (a === 169 && b === 254) return true;
if (a === 100 && b >= 64 && b <= 127) return true;
return false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isPrivateIpv4(0, 0) returns false, so the bare literal 0.0.0.0 is classified as public.

The embedded path at :208-212 already handles this exact address, with the rationale written into its own comment — "connecting to it reaches the LOCAL machine on common stacks ... private, fail closed". That reasoning never reaches here, because the v4 branch at :100-102 doesn't go through classifyIpv6 at all.

Verified on this head (Node 22, listener bound to 127.0.0.1):

new URL('https://0/').hostname  -> '0.0.0.0'
fetch('http://0:<port>/')       -> 200   // reaches the loopback listener
https://[::]/                   -> refused (correct)
https://[::ffff:0:0]/           -> refused (correct)

Suggested fix: have isPrivateIpv4 see all four octets and treat only 0.0.0.0 as private. We checked 0.0.0.1 and it does not reach a local listener, so pulling in all of 0.0.0.0/8 would widen the private set without buying anything.

…ranch

Review follow-up on apache#3502: the embedded paths classified the unspecified
address as private, but the plain-IPv4 branch never consulted that
rationale — 'https://0/' canonicalizes to 0.0.0.0, isPrivateIpv4(0, 0)
returned false, and the gate allowed a destination that verifiably
reaches a loopback-bound listener.

isPrivateIpv4 now sees all four octets and treats exactly 0.0.0.0 as
private (0.0.0.1 does not reach a local listener, so 0.0.0.0/8 stays
out). The embedded checks route through the same predicate, replacing
their separate special case. Also reformats two over-long lines from the
previous commit that failed format:check.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TMwYxgNEbz2RFmuK6AXGcj
@GabrielDrapor

Copy link
Copy Markdown
Contributor Author

Fixed at head 4e0084303 — exactly as suggested: isPrivateIpv4 now sees all four octets and treats only the exact 0.0.0.0 as private (your 0.0.0.1 probe kept 0.0.0.0/8 out). The embedded checks route through the same predicate now, replacing their separate unspecified-address special case, so the rationale lives in one place for both the plain-v4 branch and every embedding.

Regressions: core cases for 0.0.0.0 / https://0/ round-trip / 0.0.0.1 staying global, plus transport-security remote-provenance cases for https://0.0.0.0:8443/ and https://0/. Also reformatted two over-long lines from the previous commit that were failing format:check (caught it locally this time). core 612, mcp 171 — green.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving 4e00843039c049c772bfd31ca2a333b34b32f1c3. Required test is completed / success and bound to that exact SHA (run 32616240535).

The [P1] we reported is fixed, and fixed the right way. isPrivateIpv4 now takes all four octets and admits exactly 0.0.0.0:

if (a === 0 && b === 0 && c === 0 && d === 0) return true;

What I appreciate is that the comment carries the reasoning rather than the rule — including the restraint argument: 0.0.0.1 does not reach a local listener, so widening to 0.0.0.0/8 would grow the private set without buying anything. That is the part a future reader needs, because the tempting "fix" here is to be broader, and broader would have been worse. The https://0/ cases are pinned in tests, which is the spelling that actually bites in practice.

This also closes the internal inconsistency that made it a P1 rather than a nit: the file already argued at the embedded-address branch that the unspecified address must fail closed under any embedding, while the bare-v4 path did not. The two paths now agree.

The broader change — classifying IPv6 by parsed bytes instead of by string shape, with a real RFC 4291 parser — is the right call. String-prefix matching on IPv6 literals fails on exactly the spellings an attacker gets to choose.

Reviewed at 2026-08-23 12:20 UTC. No open P0–P2.

@Astro-Han
Astro-Han merged commit ebc1e18 into apache:main Aug 23, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants