From 32632fd2a6384715e57b24850ad5f56404061245 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 18:16:24 +0000 Subject: [PATCH] fix(security): close the SSRF guard's DNS oracle (CWE-209) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `assertPublicHttpUrl` threw a distinct message per rejection cause, and `transcription-service.ts` interpolated that message into `result.error`, which `/api/transcribe` returns verbatim. Since that route is unauthenticated, a caller could distinguish Rejected audioUrl: Host does not resolve Rejected audioUrl: Host resolves to a private address and enumerate internal hostnames one guess at a time, without credentials and without completing a fetch. The leak was worse than the distinct literals suggest: `dns.lookup` rejects with ENOTFOUND rather than resolving empty, so the resolver's own `getaddrinfo ENOTFOUND ` propagated out uncaught — carrying the hostname and the resolver verdict into the response body. The `resolved.length === 0` branch was effectively unreachable. Every rejection now throws `SsrfGuardError`, whose `message` is the single constant `SSRF_REJECTION_MESSAGE` and whose `reason` carries the specific cause for server-side logs only. The DNS lookup is wrapped so resolver failures become the same uniform rejection. The call site logs `reason` via `console.error` and returns the constant. Detection logic is untouched: the same URLs are accepted and rejected as before, only the reporting changed. Tests: 7 new cases in `apps/web/src/lib/__tests__/ssrf-guard.test.ts`, the first coverage this module has had. The load-bearing assertion is indistinguishability — a non-existent host and a private host must produce byte-identical caller-visible messages — because asserting merely that "a static string" is returned would still pass if the two branches returned two different static strings. Each also asserts the diagnostic survives in `reason`, so suppression is proven without proving silence. Verified non-vacuous: 6 of the 7 fail against the pre-fix guard. The 7th is the no-regression control (a public host is still allowed) and passes on both. npx tsc --noEmit clean npm run lint clean npm test 258 passed, 1 failed (47 files) The single failure is `billing-chat-gating.test.ts` "blocks free tier after daily quota" (5000 ms timeout). Confirmed pre-existing by re-running it with these changes stashed on the same checkout, where it fails identically at 5005 ms. Tracked in #1116. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ng3VUMYcPDELaNpKPgd83e --- apps/web/src/lib/__tests__/ssrf-guard.test.ts | 133 ++++++++++++++++++ apps/web/src/lib/ssrf-guard.ts | 61 +++++++- apps/web/src/lib/transcription-service.ts | 11 +- 3 files changed, 196 insertions(+), 9 deletions(-) create mode 100644 apps/web/src/lib/__tests__/ssrf-guard.test.ts diff --git a/apps/web/src/lib/__tests__/ssrf-guard.test.ts b/apps/web/src/lib/__tests__/ssrf-guard.test.ts new file mode 100644 index 000000000..b4e4f5bc4 --- /dev/null +++ b/apps/web/src/lib/__tests__/ssrf-guard.test.ts @@ -0,0 +1,133 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest'; + +/** + * Regression tests for the SSRF guard's DNS oracle (CWE-209). + * + * The guard is reached from `/api/transcribe`, an unauthenticated route that + * returns `result.error` verbatim. Before this change every rejection carried a + * distinct message, so a caller could tell "this internal hostname does not + * exist" from "this internal hostname exists and is private" purely by diffing + * the response body — enumerating internal DNS without credentials. + * + * The load-bearing assertion here is therefore *indistinguishability*: the + * caller-visible message must be byte-identical across causes. Asserting only + * that some static string is returned would pass even if the two branches + * returned two different static strings, which is the same oracle. + */ + +// `vi.hoisted` lets the mock factory close over `lookup` without hitting the +// temporal dead zone that a plain `const` would, so the module under test can +// be imported statically and keep its types. +const { lookup } = vi.hoisted(() => ({ lookup: vi.fn() })); +vi.mock('node:dns/promises', () => ({ lookup })); + +import { assertPublicHttpUrl, SsrfGuardError, SSRF_REJECTION_MESSAGE } from '@/lib/ssrf-guard'; + +/** Reject and hand back the error, failing loudly if the call unexpectedly resolved. */ +async function rejectionOf(url: string): Promise { + try { + await assertPublicHttpUrl(url); + } catch (err) { + return err as SsrfGuardError; + } + throw new Error(`Expected ${url} to be rejected, but it was allowed`); +} + +beforeEach(() => { + lookup.mockReset(); +}); + +describe('assertPublicHttpUrl — DNS oracle', () => { + it('reports a non-existent host and a private host identically', async () => { + const enotfound = Object.assign(new Error('getaddrinfo ENOTFOUND vault.corp.example'), { + code: 'ENOTFOUND', + }); + lookup.mockRejectedValueOnce(enotfound); + const missing = await rejectionOf('https://vault.corp.example/x'); + + lookup.mockResolvedValueOnce([{ address: '10.1.2.3', family: 4 }]); + const private_ = await rejectionOf('https://vault.corp.example/x'); + + // The oracle: these two must be indistinguishable to the caller. + expect(missing.message).toBe(private_.message); + expect(missing.message).toBe(SSRF_REJECTION_MESSAGE); + + // ...while the operator still gets the distinction server-side. + expect(missing.reason).not.toBe(private_.reason); + expect(missing.reason).toContain('ENOTFOUND'); + expect(private_.reason).toContain('10.1.2.3'); + }); + + it('keeps the hostname and resolver errno out of the caller-visible message', async () => { + lookup.mockRejectedValueOnce( + Object.assign(new Error('getaddrinfo EAI_AGAIN jenkins.internal.corp'), { + code: 'EAI_AGAIN', + }) + ); + const err = await rejectionOf('https://jenkins.internal.corp/'); + + expect(err.message).not.toContain('jenkins'); + expect(err.message).not.toContain('EAI_AGAIN'); + expect(err.message).not.toContain('getaddrinfo'); + // Suppressed for the caller, retained for the operator. + expect(err.reason).toContain('jenkins.internal.corp'); + expect(err.reason).toContain('EAI_AGAIN'); + }); + + it('does not leak the resolved private address to the caller', async () => { + lookup.mockResolvedValueOnce([{ address: '169.254.169.254', family: 4 }]); + const err = await rejectionOf('https://metadata.example.com/'); + + expect(err.message).not.toContain('169.254.169.254'); + expect(err.reason).toContain('169.254.169.254'); + }); + + it('gives every rejection cause the same caller-visible message', async () => { + lookup.mockResolvedValue([{ address: '10.0.0.1', family: 4 }]); + + const rejections = await Promise.all( + [ + 'not-a-url', + 'file:///etc/passwd', + 'http://localhost/', + 'http://metadata.google.internal/', + 'http://box.internal/', + 'http://box.local/', + 'http://127.0.0.1/', + 'http://169.254.169.254/', + 'http://[::1]/', + 'https://resolves-privately.example.com/', + ].map(rejectionOf) + ); + + const messages = new Set(rejections.map((r) => r.message)); + expect(messages).toEqual(new Set([SSRF_REJECTION_MESSAGE])); + + // Every one is still an SsrfGuardError carrying a distinct diagnostic. + expect(rejections.every((r) => r instanceof SsrfGuardError)).toBe(true); + expect(new Set(rejections.map((r) => r.reason)).size).toBe(rejections.length); + }); +}); + +describe('assertPublicHttpUrl — the guard still guards', () => { + it('allows a host that resolves to a public address', async () => { + lookup.mockResolvedValueOnce([{ address: '93.184.216.34', family: 4 }]); + const url = await assertPublicHttpUrl('https://example.com/audio.mp3'); + expect(url.hostname).toBe('example.com'); + }); + + it('rejects a private address hiding behind an IPv4-mapped IPv6 spelling', async () => { + lookup.mockResolvedValueOnce([{ address: '0:0:0:0:0:ffff:7f00:1', family: 6 }]); + const err = await rejectionOf('https://sneaky.example.com/'); + expect(err.reason).toContain('resolves to private address'); + }); + + it('rejects when any resolved address is private, even if another is public', async () => { + lookup.mockResolvedValueOnce([ + { address: '93.184.216.34', family: 4 }, + { address: '10.0.0.5', family: 4 }, + ]); + const err = await rejectionOf('https://mixed.example.com/'); + expect(err.reason).toContain('10.0.0.5'); + }); +}); diff --git a/apps/web/src/lib/ssrf-guard.ts b/apps/web/src/lib/ssrf-guard.ts index cc315a0cd..e60631449 100644 --- a/apps/web/src/lib/ssrf-guard.ts +++ b/apps/web/src/lib/ssrf-guard.ts @@ -12,10 +12,41 @@ import 'server-only'; * Host header. This is a strong, low-cost first line of defense. */ import * as dns from 'node:dns/promises'; +import type { LookupAddress } from 'node:dns'; import * as net from 'node:net'; const BLOCKED_HOSTNAMES = new Set(['localhost', 'metadata.google.internal']); +/** + * The single message every rejection reports to its caller. + * + * Callers of this guard sit behind unauthenticated routes and have historically + * interpolated `err.message` straight into an HTTP response. A message that + * varies by cause is therefore a DNS oracle: an attacker who can distinguish + * "does not resolve" from "resolves to a private address" can enumerate + * internal hostnames one guess at a time, without credentials and without ever + * completing a fetch. Keeping one constant here makes every rejection + * indistinguishable from outside (CWE-209). + */ +export const SSRF_REJECTION_MESSAGE = 'URL rejected: not a permitted public http(s) target'; + +/** + * Rejection carrying a uniform public `message` and a specific `reason`. + * + * `reason` is for server-side logs only — it names the host, the resolved + * address, or the resolver errno, all of which are exactly what the uniform + * message exists to withhold. Never return it to a caller. + */ +export class SsrfGuardError extends Error { + readonly reason: string; + + constructor(reason: string) { + super(SSRF_REJECTION_MESSAGE); + this.name = 'SsrfGuardError'; + this.reason = reason; + } +} + /** * True for non-public IP ranges: * IPv4 — 10/8, 172.16/12, 192.168/16 (RFC1918), 127/8 (loopback), @@ -105,23 +136,39 @@ export async function assertPublicHttpUrl(input: string): Promise { try { u = new URL(input); } catch { - throw new Error('Invalid URL'); + throw new SsrfGuardError(`Not a parseable URL: ${input}`); } if (u.protocol !== 'https:' && u.protocol !== 'http:') { - throw new Error(`Blocked URL scheme: ${u.protocol}`); + throw new SsrfGuardError(`Blocked URL scheme: ${u.protocol}`); } const host = u.hostname.toLowerCase().replace(/\.$/, ''); if (BLOCKED_HOSTNAMES.has(host) || host.endsWith('.internal') || host.endsWith('.local')) { - throw new Error('Blocked host'); + throw new SsrfGuardError(`Blocked host: ${host}`); } if (net.isIP(host)) { - if (ipIsPrivate(host)) throw new Error('Blocked private IP literal'); + if (ipIsPrivate(host)) throw new SsrfGuardError(`Blocked private IP literal: ${host}`); return u; } - const resolved = await dns.lookup(host, { all: true }); - if (resolved.length === 0) throw new Error('Host does not resolve'); + + // `dns.lookup` rejects with ENOTFOUND/EAI_AGAIN rather than resolving empty, + // so an uncaught rejection would propagate the resolver's own message + // (`getaddrinfo ENOTFOUND `) to the caller — leaking both the hostname + // and the resolver's verdict. Catching it here is what makes a + // non-existent host indistinguishable from a private one. + let resolved: LookupAddress[]; + try { + resolved = await dns.lookup(host, { all: true }); + } catch (err) { + const detail = err instanceof Error ? err.message : String(err); + throw new SsrfGuardError(`DNS lookup failed for ${host}: ${detail}`); + } + if (resolved.length === 0) { + throw new SsrfGuardError(`DNS lookup returned no addresses for ${host}`); + } for (const r of resolved) { - if (ipIsPrivate(r.address)) throw new Error('Host resolves to a private address'); + if (ipIsPrivate(r.address)) { + throw new SsrfGuardError(`Host ${host} resolves to private address ${r.address}`); + } } return u; } diff --git a/apps/web/src/lib/transcription-service.ts b/apps/web/src/lib/transcription-service.ts index 06333497c..7aa9d12f2 100644 --- a/apps/web/src/lib/transcription-service.ts +++ b/apps/web/src/lib/transcription-service.ts @@ -5,7 +5,7 @@ import { fetchYouTubeMetadata, formatMetadataAsContext } from '@/lib/youtube-met import { getGeminiClient, hasGeminiKey } from '@/lib/gemini-client'; import { GEMINI_SEARCH_MODEL } from '@/lib/gemini-models'; import { gatewayChat, hasAiGatewayKey, toGatewayModelId } from '@/lib/vercel-ai-gateway'; -import { assertPublicHttpUrl } from '@/lib/ssrf-guard'; +import { assertPublicHttpUrl, SsrfGuardError, SSRF_REJECTION_MESSAGE } from '@/lib/ssrf-guard'; let _openai: OpenAI | null = null; function getOpenAI() { @@ -273,9 +273,16 @@ ${metadataContext ? `\nKNOWN METADATA:\n${metadataContext}` : ''}`, try { await assertPublicHttpUrl(audioUrl); } catch (guardErr) { + // The specific cause names the host, its resolved address, or the + // resolver errno. That belongs in the logs, not in a response body on + // an unauthenticated route — see SSRF_REJECTION_MESSAGE. + console.error( + '[transcription] audioUrl rejected by SSRF guard:', + guardErr instanceof SsrfGuardError ? guardErr.reason : guardErr + ); return { success: false, - error: `Rejected audioUrl: ${guardErr instanceof Error ? guardErr.message : 'blocked'}`, + error: `Rejected audioUrl: ${SSRF_REJECTION_MESSAGE}`, transcript: '', }; }