Skip to content

SSRF guard: neither IPv6 bit-mask range has its boundaries pinned #1520

Description

@groupthinking

Problem

ipIsPrivate in apps/web/src/lib/ssrf-guard.ts blocks two IPv6 ranges using bit masks rather than exact matches:

if ((h[0] & 0xffc0) === 0xfec0) return true;                                  // fec0::/10 site-local
if (h[0] === 0x0100 && h[1] === 0 && h[2] === 0 && h[3] === 0) return true;   // 100::/64 discard

ssrf-guard.test.ts (added by #1486) covers the too-narrow direction well — every blocked range has an address squarely inside it, so a check that stopped matching would fail a test.

Nothing covers the too-wide direction. Neither edge of either mask is pinned, and no test asserts that an address just outside the range is still allowed. A widened mask would pass the entire existing suite.

Why it matters

A too-wide mask fails silently and asymmetrically. It refuses legitimate public destinations, and the only symptom is a rejection carrying the same uniform message every other rejection uses — the indistinguishability property #1381 deliberately introduced to close the DNS oracle. There is no distinct error, no log line that says "this was a mask boundary", nothing separating "correctly blocked" from "wrongly blocked" at the call site.

Both masks have a plausible slip within one character of the current code:

  • fec0::/10fe00::/8. Both start fe; 0xffc0/0xfec0 vs 0xff00/0xfe00. This would start refusing fe00::/8 space that neither fe80::/10 nor fec0::/10 covers.
  • 100::/64100::/16. Dropping the h[1]h[3] conjunction leaves h[0] === 0x0100, which refuses all of 100::/16 rather than the RFC 6666 discard /64.

Acceptance criteria

  • Both edges of fec0::/10 are asserted rejected (first and last address).
  • The first address of 100::/64 is asserted rejected.
  • A just-outside neighbour of each range is asserted allowed — at minimum fe00::1, 101::1, and 100:0:0:1::1.
  • Coverage runs through both paths into ipIsPrivate: URL literals and DNS answers. They enter via different branches of assertPublicHttpUrl and report different messages, so a regression can land on one and not the other.
  • Non-vacuity is demonstrated by mutation, not by failing against main — there is no bug here, so these tests pass on main by construction. Each must be shown to fail against a deliberately widened mask.

Out of scope

Detection logic. main is correct as of #1486; this is purely about holding it correct.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions