Skip to content

Close the DNS-rebinding gap by pinning outbound sockets to the validated address - #8

Merged
GunsNR merged 2 commits into
mainfrom
claude/dns-rebinding-socket-pinning-ar8ilh
Aug 28, 2026
Merged

GunsNR merged 2 commits into
mainfrom
claude/dns-rebinding-socket-pinning-ar8ilh

Conversation

@GunsNR

@GunsNR GunsNR commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Closes the last open SSRF item, recorded until now as a known residual risk in docs/operations-runbook.md §4 and docs/release-truth-audit.md.

The vulnerability

safeFetch validated a hostname and then handed that hostname to fetch, which performs its own second DNS resolution. Between the two lookups the answer can change:

  1. checkResolvedHost() resolves attacker.example → 93.184.216.34 → allowed.
  2. fetch('https://attacker.example/') resolves again → 169.254.169.254.
  3. The socket opens on the metadata service. Every check the guard performed described an address the request never used.

A zero-TTL record is all an attacker needs, and they only need to control DNS for a name they own. The validated address never reached the socket layer — that was the entire defect.

Three further bypasses found while fixing it

Not previously documented, all in the literal guard:

  • Non-canonical IPv4. The only IPv4 recognizer was /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/. 127.1, 0177.0.0.1, 0x7f000001 and 2130706433 are not matched, contain a dot (or no colon), and fell through to return { allowed: true } as ordinary hostnames. Confirmed against dns.lookup: all four reach 127.0.0.1.
  • IPv4-mapped IPv6 was checked in a form that never occurs. The check matched ::ffff:127.0.0.1, but new URL('http://[::ffff:127.0.0.1]/').hostname returns [::ffff:7f00:1]. Since parsed URLs are the only way these reach the guard, the check could not fire. 0:0:0:0:0:ffff:169.254.169.254 normalizes to ::ffff:a9fe:a9fe and was likewise allowed.
  • IPv6 was allow-by-default outside four prefixes. NAT64, 6to4, Teredo, ORCHIDv2, 2001:db8::/32, multicast and site-local all passed.

Plus missing IPv4 ranges (192.0.0.0/24, 192.0.2.0/24, 192.88.99.0/24, 198.18.0.0/15, 198.51.100.0/24, 203.0.113.0/24) and Azure's 168.63.129.16, which is globally routable and so was caught by nothing.

The fix

src/lib/net-pinned.ts (new) — the transport. node:http/node:https with a per-request lookup that ignores the hostname it is handed and returns the one approved address. There is no second lookup to poison because there is no second lookup. Node's fetch cannot express this: the dispatcher option that would allow it needs an undici dependency the project does not carry.

The hostname is deliberately not pinned. options.host stays the name, so TLS SNI, certificate validation and the Host header are unchanged; only the TCP peer is substituted. Rewriting the URL to the IP — the obvious shortcut — silently disables certificate validation and trades an SSRF hole for a transport-security hole. A post-connect assertion compares socket.remoteAddress against the pin and hangs up on mismatch.

src/lib/ip-address.ts (new) — parses to bytes and classifies bytes. Loose IPv4 (inet_aton semantics: 1–4 parts, decimal/octal/hex) and full IPv6 (:: compression, embedded IPv4, zone IDs, RFC 5952 canonicalization). IPv6 is allow-listed to global unicast 2000::/3 minus carve-outs, rather than block-listed — the space is too large to enumerate what is unsafe. Embedded-IPv4 forms are recursively classified as the IPv4 address they carry.

src/lib/net-guard.ts — delegates to the parser; exported signatures unchanged, so both route call sites are untouched. A host that is trying to be an address and failing (999.1.1.1) is now refused rather than demoted to a hostname.

src/lib/net-fetch.ts — resolve once → validate every record → pin the first → connect. Repeated per redirect hop, each pinned independently. One timeout budget across the whole chain rather than one per hop, and a byte cap passed to the transport.

Bodies are read under a cap applied to the encoded and decoded streams alike — a compression bomb exhausts memory long before any decoded limit is consulted.

allowPrivateHosts disables validation only; the pin still applies. A request whose destination is unknowable is not made safer by skipping a check.

Test coverage

74 tests across four files, +59 net.

File Covers
tests/net-pinned.test.ts (13, new) Every request is addressed to pinned.invalid, a name RFC 2606 guarantees never resolves — a response therefore proves no DNS lookup occurred. Host header preservation, TLS SNI and host options, pinned-lookup return values, peer-address mismatch, byte cap, decompressed cap, deadline, connection failure, 3xx not followed
tests/ip-address.test.ts (14, new) Every notation above, RFC 5952 canonicalization including the leftmost-run tie rule, malformed input, all blocked ranges, and adjacent-range non-over-blocking
tests/net-fetch.test.ts (27, was 15) All prior assertions preserved and re-pointed at the transport seam, plus: a rebinding resolver that answers public then metadata — asserts it is asked once and the socket gets the checked address; per-hop pinning; total timeout budget
tests/net-guard.test.ts (28, was 15) All 15 prior tests pass unchanged; +13 for the newly closed spellings and ranges

tests/crawler.integration.test.ts and tests/wordpress.integration.test.ts exercise the new transport end to end against real fixture servers and were not modified.

Verification

Run locally against PostgreSQL 16, matching the CI job:

  • npm run typecheck — clean
  • npm run lint — clean
  • npm test — 663 passed / 663, 39 files (previously 576 with 87 skipped for want of a database)
  • npm run build — compiled successfully
  • npm run db:rehearse — passed
  • prisma migrate deploy + drift check — no drift

Scope

No capability status changed. src/lib/capabilities.ts and src/lib/roadmap.ts are untouched, no flag was activated, nothing was deployed, Railway was not modified, and Phase 2 is not marked complete. This satisfies one phase-2 acceptance criterion — "SSRF protection resolves DNS and defends against redirects and rebinding" — and leaves the phase's other criteria as they were.

Documentation updated to match: the runbook's residual-risk list (renumbered), the release truth audit (recorded as closed-after-Phase-2 rather than edited out of history), and ADR-016.

Remaining risks

  • The pin covers the addresses a resolver returns at check time. A host whose legitimate address changes mid-request now fails rather than following — correct, but it is a behaviour change for very-low-TTL load balancers.
  • Only the first validated address is pinned; there is no failover to the second if it is unreachable. Fail-closed by choice, and worth revisiting if a multi-homed customer site proves flaky.
  • Response bodies are buffered, not streamed. Bounded by maxBytes (5 MiB default), but a large crawl holds that per in-flight request.
  • HTTP/2 and keep-alive pooling are not used; each request opens its own connection. Correctness over throughput, and reconsidering it means re-deriving the pin per pooled socket.

Generated by Claude Code

claude added 2 commits August 28, 2026 17:39
Validating a hostname and then handing that hostname to `fetch` is a
time-of-check/time-of-use bug. `fetch` resolves the name a second time, so
an attacker serving a zero-TTL record answers the guard's lookup with a
public address and the client's lookup with 169.254.169.254. Every check
the guard performed described an address the request never used. This was
recorded as a known residual risk rather than fixed; it is now fixed.

Outbound requests resolve once through a controlled resolver, validate
every address returned, and open the connection to the validated address
via a per-request `lookup` on node:http/node:https. Node's `fetch` cannot
express this without an `undici` dependency the project does not carry.
The hostname is deliberately not pinned: `options.host` stays the name, so
TLS SNI, certificate validation and the Host header are unchanged and only
the TCP peer is substituted. Rewriting the URL to the IP — the obvious
shortcut — would trade an SSRF hole for a transport-security hole.

Three bypasses in the literal guard are closed alongside it. The address
check read exactly one spelling of an address, a dotted quad, so `127.1`,
`0177.0.0.1`, `0x7f000001` and `2130706433` fell through as ordinary
hostnames despite reaching loopback through getaddrinfo. So did
`::ffff:7f00:1` — the IPv4-mapped form the WHATWG URL parser actually
produces, meaning the old mapped-address check could not fire on a parsed
URL. Addresses are now parsed to bytes and classified as bytes in every
notation a resolver accepts; IPv6 is allow-listed to global unicast rather
than block-listed; and 192.0.0.0/24, 192.0.2.0/24, 192.88.99.0/24,
198.18.0.0/15, 198.51.100.0/24, 203.0.113.0/24 and Azure's globally
routable 168.63.129.16 are refused.

Bodies are read under a byte cap applied to the encoded and decoded
streams alike, since a compression bomb exhausts memory before any decoded
limit is consulted, and the redirect chain now spends one timeout budget
rather than one per hop.

No capability status changed and no roadmap phase moved. This closes one
phase-2 acceptance criterion and leaves the rest as they were.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQ2eShwv2iVM3CQYpjEkxK
Pinning changed multi-address behaviour and the change was not written
down. Previously every resolved address was validated and the hostname was
handed to `fetch`, which chose an address and could fall back to another if
the first refused. Now every address is still validated but exactly one is
connected to, with no fallback.

That is the deliberate cost of pinning — falling back would mean connecting
to an address chosen after the check, which is the hole the pin closes — but
it is a real operational difference for a multi-homed host, so it belongs in
the runbook's residual-risk list and in ADR-016's consequences rather than
only in a pull request description.

Documentation only. No code, capability or roadmap change.

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

GunsNR commented Aug 28, 2026

Copy link
Copy Markdown
Owner Author

Pre-merge review — findings

Read-only review against eight criteria. No blockers. One documentation gap found and closed in 0d50acd (docs only, no code).

Three behaviours below are worth recording. All three exist identically on main today — this PR carries them forward rather than introducing them — and none is a bypass of the pin itself.

1. A POST body survives a cross-origin redirect

Credentials are stripped when a redirect crosses origin, but the method and body are not. Verified with a stub transport:

hop 1  POST https://example.com/     body="secret-post-body"  Authorization: Bearer S
hop 2  POST https://other.example/   body="secret-post-body"  (Authorization stripped)

This matters most for publishPost, where the body is article content and the site URL is customer-supplied: a compromised WordPress install could 302 to an attacker host and receive the payload. main behaves the same way — the previous implementation spread init (body included) into every hop's fetch — so this is not a regression, and fixing it means deciding whether to drop the body cross-origin or refuse the redirect outright. Worth its own change rather than widening this one.

Related: on a 303, the method stays POST rather than becoming GET. RFC 9110 requires the change. Same pre-existing origin, same fix.

2. Proxy-Authorization is not in the strip list

CREDENTIAL_HEADERS covers authorization, cookie and x-supertool-key. A Proxy-Authorization header would survive a cross-origin redirect. No call site sends one and nothing in the codebase configures a proxy, so there is no live exposure — but it is a one-line hardening worth taking.

3. DNS resolution is inside the timeout accounting, not inside its bound

The shared deadline is consulted after resolveAndPin returns, so resolution time is charged against the budget but a hanging lookup is not interrupted by it. Measured: a 400 ms resolver against a 100 ms budget took 401 ms before the deadline check fired. In production dns.lookup is bounded by the OS resolver rather than by us. main has no resolver timeout either.

What verified clean

  • TLS/SNI/Host — options.host and servername both carry the hostname; SNI omitted for IP literals; Host is url.host. Covered by tests/net-pinned.test.ts.
  • remoteAddress normalization — 9 cases checked, including ::ffff:127.0.0.1 ≡ 127.0.0.1, the hex form ::ffff:7f00:1, uncompressed IPv6, and unparseable input failing closed.
  • HTTPS→HTTP downgrade — treated as cross-origin (scheme is part of origin), so credentials are stripped. Unchanged from main.
  • Shared timeout across hops — per-hop budgets observed shrinking 5000 → 4969 → 4938; covers connection and body reading.
  • Both body caps fail closed — encoded and decoded counters each destroy the stream and reject.
  • All-or-nothing address validation — any private, reserved or unparseable record rejects the whole name.
  • No alternate outbound path — every server-side fetch call targets a hardcoded provider endpoint (api.openai.com, api.anthropic.com, api.x.ai, api.perplexity.ai, Gemini, DataForSEO, Resend, Postmark). The only user-supplied-URL paths are wordpress.ts and seo/crawler.ts, both via safeFetch. No axios/got/undici/node-fetch, no raw http.request outside the transport.
  • Proxy behaviour — the app reads no proxy environment variable and sets no dispatcher or ProxyAgent; neither fetch nor node:http honours proxy env vars by default. No change.

The gap that was closed

Multi-address handling did change: main validated every address then handed the hostname to fetch, which chose an address and could fall back to another. Pinning removes that fallback. That was documented only in this PR description, so 0d50acd records it in the runbook's residual-risk list and in ADR-016's consequences.


Generated by Claude Code

@GunsNR
GunsNR marked this pull request as ready for review August 28, 2026 19:57
@GunsNR
GunsNR merged commit 258a375 into main Aug 28, 2026
2 checks 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