diff --git a/src/app/api/integrations/moodlog/test/route.ts b/src/app/api/integrations/moodlog/test/route.ts index 35eb80ed5..36b20a09e 100644 --- a/src/app/api/integrations/moodlog/test/route.ts +++ b/src/app/api/integrations/moodlog/test/route.ts @@ -116,7 +116,9 @@ export const POST = apiHandler(async (request: NextRequest) => { try { // safeFetch keeps the manual-redirect guard (the apiKey would // otherwise leak on a 302 to 169.254.169.254 / RFC1918) and adds - // the AbortSignal.timeout convention. + // the AbortSignal.timeout convention. requirePublicHost layers in + // the connect-time IP pin so a DNS rebinding cannot flip the host + // between accept and dispatch (issue #217). const res = await safeFetch( probeUrl.toString(), { @@ -124,7 +126,7 @@ export const POST = apiHandler(async (request: NextRequest) => { headers: { Authorization: `Bearer ${apiKey}` }, cache: "no-store", }, - { timeoutMs: TIMEOUT_MS }, + { timeoutMs: TIMEOUT_MS, requirePublicHost: true }, ); const latencyMs = Math.round(performance.now() - start); diff --git a/src/lib/__tests__/safe-fetch-dispatcher.test.ts b/src/lib/__tests__/safe-fetch-dispatcher.test.ts new file mode 100644 index 000000000..c2c455ce8 --- /dev/null +++ b/src/lib/__tests__/safe-fetch-dispatcher.test.ts @@ -0,0 +1,170 @@ +import { describe, it, expect, afterEach, vi } from "vitest"; +import dns from "node:dns"; +import { isPublicIp } from "@/lib/validations/notifications"; +import { + getPinnedPublicDispatcher, + _resetPinnedDispatcherForTests, +} from "../safe-fetch-dispatcher"; + +describe("isPublicIp", () => { + it("accepts a public IPv4 address", () => { + expect(isPublicIp("203.0.113.5")).toBe(true); + expect(isPublicIp("8.8.8.8")).toBe(true); + }); + + it("rejects RFC1918 IPv4", () => { + expect(isPublicIp("10.0.0.1")).toBe(false); + expect(isPublicIp("192.168.0.1")).toBe(false); + expect(isPublicIp("172.16.0.1")).toBe(false); + }); + + it("rejects loopback + reserved IPv4", () => { + expect(isPublicIp("127.0.0.1")).toBe(false); + expect(isPublicIp("0.0.0.0")).toBe(false); + }); + + it("rejects 169.254/16 cloud-metadata", () => { + expect(isPublicIp("169.254.169.254")).toBe(false); + }); + + it("rejects CGNAT 100.64/10", () => { + expect(isPublicIp("100.64.0.1")).toBe(false); + expect(isPublicIp("100.127.255.255")).toBe(false); + }); + + it("rejects IPv6 loopback + link-local + ULA", () => { + expect(isPublicIp("::1")).toBe(false); + expect(isPublicIp("fe80::1")).toBe(false); + expect(isPublicIp("fc00::1")).toBe(false); + expect(isPublicIp("fd12:3456:789a::1")).toBe(false); + }); + + it("accepts a public IPv6 address", () => { + expect(isPublicIp("2001:db8::1")).toBe(true); + expect(isPublicIp("2606:4700:4700::1111")).toBe(true); + }); + + it("rejects IPv4-mapped IPv6 that wraps a private IPv4", () => { + expect(isPublicIp("::ffff:127.0.0.1")).toBe(false); + expect(isPublicIp("::ffff:10.0.0.1")).toBe(false); + }); + + it("accepts IPv4-mapped IPv6 that wraps a public IPv4", () => { + expect(isPublicIp("::ffff:8.8.8.8")).toBe(true); + }); + + it("rejects empty input", () => { + expect(isPublicIp("")).toBe(false); + }); +}); + +describe("pinnedPublicDispatcher", () => { + afterEach(() => { + vi.restoreAllMocks(); + _resetPinnedDispatcherForTests(); + }); + + it("refuses a hostname that resolves to a private IP", async () => { + // Mock dns.lookup to return 169.254.169.254 (cloud metadata). + const lookupSpy = vi.spyOn(dns, "lookup"); + lookupSpy.mockImplementation( + (( + _hostname: string, + _opts: dns.LookupAllOptions, + callback: ( + err: NodeJS.ErrnoException | null, + addresses: dns.LookupAddress[], + ) => void, + ) => { + callback(null, [{ address: "169.254.169.254", family: 4 }]); + }) as unknown as typeof dns.lookup, + ); + + const dispatcher = getPinnedPublicDispatcher(); + // Drive the dispatcher through `fetch` so the integration mirrors + // what safeFetch does. The expected outcome is a connect error + // (the lookup wrapper raised ENOTFOUND). + await expect( + fetch( + "https://attacker-controlled.example.test/probe", + // Node's fetch RequestInit accepts undici's `dispatcher`; the + // DOM types do not, hence the cast. + { dispatcher } as RequestInit & { dispatcher: typeof dispatcher }, + ), + ).rejects.toThrow(); + }, 20_000); + + it("forwards a public-resolving hostname to the real connector", async () => { + // Mock dns.lookup to claim the host resolves to 192.0.2.1 — a + // documented public range (TEST-NET-1) that is also unroutable. + // The expected outcome is therefore NOT ENOTFOUND but a connect + // failure further down the stack (the IP refuses or times out). + const lookupSpy = vi.spyOn(dns, "lookup"); + lookupSpy.mockImplementation( + (( + _hostname: string, + _opts: dns.LookupAllOptions, + callback: ( + err: NodeJS.ErrnoException | null, + addresses: dns.LookupAddress[], + ) => void, + ) => { + callback(null, [{ address: "192.0.2.1", family: 4 }]); + }) as unknown as typeof dns.lookup, + ); + + const dispatcher = getPinnedPublicDispatcher(); + let caught: unknown; + try { + await fetch("https://example.test/probe", { + signal: AbortSignal.timeout(500), + dispatcher, + } as RequestInit & { dispatcher: typeof dispatcher }); + } catch (e) { + caught = e; + } + + // The pinned lookup must have ACCEPTED the address (no ENOTFOUND); + // the connect failure surfaces as a timeout / connect refused. + expect(caught).toBeDefined(); + const code = (caught as NodeJS.ErrnoException)?.code; + expect(code === "ENOTFOUND").toBe(false); + }, 20_000); + + it("filters a mixed result so only the public address is pinned", async () => { + // Mock dns.lookup to return one private + one public address. The + // dispatcher must drop the private one and pin to the public one. + const lookupSpy = vi.spyOn(dns, "lookup"); + lookupSpy.mockImplementation( + (( + _hostname: string, + _opts: dns.LookupAllOptions, + callback: ( + err: NodeJS.ErrnoException | null, + addresses: dns.LookupAddress[], + ) => void, + ) => { + callback(null, [ + { address: "10.0.0.5", family: 4 }, + { address: "192.0.2.1", family: 4 }, + ]); + }) as unknown as typeof dns.lookup, + ); + + const dispatcher = getPinnedPublicDispatcher(); + let caught: unknown; + try { + await fetch("https://example.test/probe", { + signal: AbortSignal.timeout(500), + dispatcher, + } as RequestInit & { dispatcher: typeof dispatcher }); + } catch (e) { + caught = e; + } + + // Again: the dispatch must NOT short-circuit with ENOTFOUND — the + // public alternate is what gets dialled. + const code = (caught as NodeJS.ErrnoException)?.code; + expect(code === "ENOTFOUND").toBe(false); + }, 20_000); +}); diff --git a/src/lib/moodlog/push.ts b/src/lib/moodlog/push.ts index a4c2382a7..32363bdbe 100644 --- a/src/lib/moodlog/push.ts +++ b/src/lib/moodlog/push.ts @@ -114,6 +114,10 @@ export async function pushMoodEntriesToMoodLog( const start = performance.now(); let response: Response; try { + // requirePublicHost adds the DNS-rebinding pin to the input-time + // isPublicUrl guard (issue #217). The reverse-sync path attaches a + // user-supplied bearer; a rebinding flip would leak it on the + // connect call to the swapped private address. response = await safeFetch( url.toString(), { @@ -124,7 +128,7 @@ export async function pushMoodEntriesToMoodLog( }, body: JSON.stringify(body), }, - { timeoutMs: 10_000 }, + { timeoutMs: 10_000, requirePublicHost: true }, ); } catch (err) { const message = err instanceof Error ? err.message : String(err); diff --git a/src/lib/moodlog/sync.ts b/src/lib/moodlog/sync.ts index ccd238057..b23e3b64b 100644 --- a/src/lib/moodlog/sync.ts +++ b/src/lib/moodlog/sync.ts @@ -90,7 +90,10 @@ export async function syncMoodLogEntries( // safeFetch keeps the no-redirect-follow guard (the apiKey would // otherwise leak on an attacker-controlled 302 hop) and adds a hard - // upper bound via AbortSignal.timeout. + // upper bound via AbortSignal.timeout. requirePublicHost routes the + // dial through the pinned-IP dispatcher so a DNS rebinding cannot + // flip a public hostname to 169.254.169.254 / RFC1918 between the + // input-time accept and the connect call. const timeoutMs = opts?.fullSync ? 60_000 : 15_000; let response: Response; const fetchStart = performance.now(); @@ -98,7 +101,7 @@ export async function syncMoodLogEntries( response = await safeFetch( url.toString(), { headers: { Authorization: `Bearer ${apiKey}` } }, - { timeoutMs }, + { timeoutMs, requirePublicHost: true }, ); } catch (err) { getEvent()?.addExternalCall({ diff --git a/src/lib/notifications/senders/ntfy.ts b/src/lib/notifications/senders/ntfy.ts index 608cb8c43..05248d381 100644 --- a/src/lib/notifications/senders/ntfy.ts +++ b/src/lib/notifications/senders/ntfy.ts @@ -39,6 +39,11 @@ export async function sendViaNtfy( // Strip HTML tags for ntfy (plain text only) const body = payload.message.replace(/<[^>]*>/g, ""); + // requirePublicHost adds the DNS-rebinding pin (issue #217) on top + // of the input-time isPublicUrl guard already enforced when the + // user saved the ntfy serverUrl in their channel config. The + // authToken would otherwise leak on a rebinding flip to a private + // address. const res = await safeFetch( url, { @@ -46,7 +51,7 @@ export async function sendViaNtfy( headers, body, }, - { timeoutMs: 5_000 }, + { timeoutMs: 5_000, requirePublicHost: true }, ); getEvent()?.addExternalCall({ diff --git a/src/lib/safe-fetch-dispatcher.ts b/src/lib/safe-fetch-dispatcher.ts new file mode 100644 index 000000000..1919674cb --- /dev/null +++ b/src/lib/safe-fetch-dispatcher.ts @@ -0,0 +1,106 @@ +import dns from "node:dns"; +import { Agent } from "undici"; +import { isPublicIp } from "@/lib/validations/notifications"; + +/** + * Custom `undici.Agent` that resolves the request hostname literally, + * vets every returned address against `isPublicIp`, and pins the + * connection to the first allowed address. + * + * This closes the DNS-rebinding gap that `isPublicUrl` cannot reach + * (issue #217). The input-time check accepts a hostname that resolves + * to a public address; the resolver is then free to flip the record to + * `169.254.169.254` (cloud metadata) or `10.0.0.x` (operator admin + * panel) by the time the connection dials. With a TTL of 0 the attack + * is reliable. + * + * Mechanism: + * + * 1. Override `connect.lookup` with a wrapper that always runs + * `dns.lookup({ all: true })` so we see every address the resolver + * would have returned. + * 2. Filter that list through `isPublicIp` — same allowlist + * `isPublicUrl` enforces, applied to a literal IP address rather + * than the input string. + * 3. If at least one address passes, hand the single first survivor + * to undici. Undici then pins the TCP connect to that exact IP, + * side-stepping any further resolver hop. + * 4. If no address passes, fail closed with `dns.NOTFOUND` so the + * dispatch surfaces as a normal connect error rather than a silent + * bypass. + * + * Wired into `safeFetch` only when `opts.requirePublicHost` is true. + * The five outbound paths that accept a user-supplied host all use + * that flag. + */ +function pinnedLookup( + hostname: string, + options: dns.LookupOptions, + callback: ( + err: NodeJS.ErrnoException | null, + address: string | dns.LookupAddress[], + family?: number, + ) => void, +): void { + // Always ask for every address so the filter sees the full set; + // discarding alternates a low-TTL attacker could swap in is exactly + // the point of the pin. + const opts: dns.LookupAllOptions = { + family: options.family ?? 0, + hints: options.hints, + verbatim: options.verbatim, + all: true, + }; + + dns.lookup(hostname, opts, (err, addresses) => { + if (err) { + callback(err, ""); + return; + } + const allowed = addresses.filter((a) => isPublicIp(a.address)); + if (allowed.length === 0) { + const refused: NodeJS.ErrnoException = new Error( + `safeFetch refused private resolved address for ${hostname}`, + ); + refused.code = "ENOTFOUND"; + callback(refused, ""); + return; + } + + const picked = allowed[0]; + if (options.all) { + // Caller asked for the array shape — pass through the survivors. + callback(null, allowed); + return; + } + callback(null, picked.address, picked.family); + }); +} + +/** + * Lazily-instantiated singleton. `undici.Agent` keeps an internal pool + * per origin, so reusing one instance across calls preserves the keep- + * alive benefit while still pinning each new connection through the + * vetted lookup. + */ +let cached: Agent | null = null; + +export function getPinnedPublicDispatcher(): Agent { + if (cached) return cached; + cached = new Agent({ + connect: { + lookup: pinnedLookup, + }, + }); + return cached; +} + +/** + * Test helper. The dispatcher is module-state; tests that mock + * `dns.lookup` need to clear the cached instance so the next call + * binds the mock. + */ +export function _resetPinnedDispatcherForTests(): void { + cached?.destroy().catch(() => {}); + cached = null; +} diff --git a/src/lib/safe-fetch.ts b/src/lib/safe-fetch.ts index 86b0754b5..af06ec094 100644 --- a/src/lib/safe-fetch.ts +++ b/src/lib/safe-fetch.ts @@ -1,4 +1,5 @@ import { isPublicUrl } from "@/lib/validations/notifications"; +import { getPinnedPublicDispatcher } from "@/lib/safe-fetch-dispatcher"; /** * Defaults applied to every `safeFetch` call. Centralised so the values @@ -115,8 +116,23 @@ export async function safeFetch( opts.timeoutMs ?? DEFAULT_TIMEOUT_MS, ); + // When the caller asks for the public-host guard, also route through + // the pinned-IP dispatcher so a DNS rebinding cannot flip the host + // between the input-time `isPublicUrl` accept and the connect call. + // Node's `fetch` accepts an undici `dispatcher` on RequestInit; the + // DOM types don't reflect that, so we cast at the boundary. + const dispatcher = opts.requirePublicHost + ? getPinnedPublicDispatcher() + : undefined; + const finalInit = { + ...init, + redirect, + signal, + ...(dispatcher ? { dispatcher } : {}), + } as RequestInit; + try { - return await fetch(target, { ...init, redirect, signal }); + return await fetch(target, finalInit); } catch (err) { // `AbortSignal.timeout` aborts with `TimeoutError` (`DOMException` // named "TimeoutError"); a caller-side abort surfaces as diff --git a/src/lib/validations/notifications.ts b/src/lib/validations/notifications.ts index 8f3f563fe..55d420063 100644 --- a/src/lib/validations/notifications.ts +++ b/src/lib/validations/notifications.ts @@ -61,6 +61,74 @@ function isPrivateIpv4(ip: [number, number, number, number]): boolean { return false; } +/** + * Decide whether a raw IP address string (as returned by `dns.lookup`) + * points at a public range — i.e. is safe to dial. + * + * Used by the pinned dispatcher (issue #217) inside undici's connect + * hook to defeat DNS rebinding: even when `isPublicUrl` accepted the + * hostname at input time, the resolver can flip between accept and + * dispatch. This second check runs against the literal address the + * connection would target. + * + * Accepts: + * - dotted-quad IPv4 ("203.0.113.5", "10.0.0.1") + * - IPv6 literal ("2001:db8::1", "::1", "fe80::1", "::ffff:127.0.0.1") + * + * Rejects every range `isPublicUrl` would reject at input time plus + * IPv4-mapped IPv6 (resolvers occasionally emit these). + */ +export function isPublicIp(ip: string): boolean { + if (!ip) return false; + const lower = ip.toLowerCase(); + + // IPv4 dotted-quad (what dns.lookup with family=4 returns). + if (/^\d+\.\d+\.\d+\.\d+$/.test(lower)) { + const parsed = parseIpv4Strict(lower); + if (!parsed) return false; + return !isPrivateIpv4(parsed); + } + + // IPv6. Only strings with a colon make it past the previous check. + if (lower.includes(":")) { + if (lower === "::1" || lower === "::") return false; + if (lower.startsWith("fe80:")) return false; + if (/^fc[0-9a-f]{0,2}:/.test(lower)) return false; + if (/^fd[0-9a-f]{0,2}:/.test(lower)) return false; + + // IPv4-mapped IPv6 dotted form ("::ffff:127.0.0.1"). Some resolvers + // emit this when an AAAA falls through to a synthesized A. + const mappedDotted = lower.match(/^(?:::ffff:)(\d+\.\d+\.\d+\.\d+)$/); + if (mappedDotted) { + const parsed = parseIpv4Strict(mappedDotted[1]); + if (!parsed || isPrivateIpv4(parsed)) return false; + return true; + } + + // IPv4-mapped IPv6 hex pair form ("::ffff:7f00:1"). + const mappedHex = lower.match( + /^(?:::ffff:)([0-9a-f]{1,4}):([0-9a-f]{1,4})$/, + ); + if (mappedHex) { + const high = parseInt(mappedHex[1], 16); + const low = parseInt(mappedHex[2], 16); + const parsed: [number, number, number, number] = [ + (high >> 8) & 0xff, + high & 0xff, + (low >> 8) & 0xff, + low & 0xff, + ]; + if (isPrivateIpv4(parsed)) return false; + return true; + } + return true; + } + + // Anything else (non-IP string, malformed) — refuse rather than + // dispatch. + return false; +} + export function isPublicUrl(url: string): boolean { try { // Pre-URL guard #1: the WHATWG URL parser interprets leading-zero IPv4