From 10329e918f7adae9e3bddeb27db2832570ec70cd Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 18 Mar 2026 23:46:48 -0700 Subject: [PATCH] fix: harden URL validation against hostname bypasses (Codex P1) Codex review found that metadata IPs could be reached via hex (0xA9FEA9FE), decimal (2852039166), octal, trailing dot, and IPv6 bracket forms. Now normalizes hostnames before checking the blocklist and probes numeric IP representations via URL constructor. Also moves URL validation before page allocation in newTab() to prevent zombie tabs on rejection (Codex P3). 5 new test cases for bypass variants. Co-Authored-By: Claude Opus 4.6 (1M context) --- browse/src/browser-manager.ts | 6 +++- browse/src/url-validation.ts | 45 ++++++++++++++++++++++++++---- browse/test/url-validation.test.ts | 20 +++++++++++++ 3 files changed, 65 insertions(+), 6 deletions(-) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index bf26ede6..31a1f9de 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -120,6 +120,11 @@ export class BrowserManager { async newTab(url?: string): Promise { if (!this.context) throw new Error('Browser not launched'); + // Validate URL before allocating page to avoid zombie tabs on rejection + if (url) { + validateNavigationUrl(url); + } + const page = await this.context.newPage(); const id = this.nextTabId++; this.pages.set(id, page); @@ -129,7 +134,6 @@ export class BrowserManager { this.wirePageEvents(page); if (url) { - validateNavigationUrl(url); await page.goto(url, { waitUntil: 'domcontentloaded', timeout: 15000 }); } diff --git a/browse/src/url-validation.ts b/browse/src/url-validation.ts index 7b181320..1ce8c45b 100644 --- a/browse/src/url-validation.ts +++ b/browse/src/url-validation.ts @@ -3,11 +3,45 @@ * Localhost and private IPs are allowed (primary use case: QA testing local dev servers). */ -const BLOCKED_METADATA_HOSTS = [ +const BLOCKED_METADATA_HOSTS = new Set([ '169.254.169.254', // AWS/GCP/Azure instance metadata 'fd00::', // IPv6 unique local (metadata in some cloud setups) 'metadata.google.internal', // GCP metadata -]; +]); + +/** + * Normalize hostname for blocklist comparison: + * - Strip trailing dot (DNS fully-qualified notation) + * - Strip IPv6 brackets (URL.hostname includes [] for IPv6) + * - Resolve hex (0xA9FEA9FE) and decimal (2852039166) IP representations + */ +function normalizeHostname(hostname: string): string { + // Strip IPv6 brackets + let h = hostname.startsWith('[') && hostname.endsWith(']') + ? hostname.slice(1, -1) + : hostname; + // Strip trailing dot + if (h.endsWith('.')) h = h.slice(0, -1); + return h; +} + +/** + * Check if a hostname resolves to the link-local metadata IP 169.254.169.254. + * Catches hex (0xA9FEA9FE), decimal (2852039166), and octal (0251.0376.0251.0376) forms. + */ +function isMetadataIp(hostname: string): boolean { + // Try to parse as a numeric IP via URL constructor — it normalizes all forms + try { + const probe = new URL(`http://${hostname}`); + const normalized = probe.hostname; + if (BLOCKED_METADATA_HOSTS.has(normalized)) return true; + // Also check after stripping trailing dot + if (normalized.endsWith('.') && BLOCKED_METADATA_HOSTS.has(normalized.slice(0, -1))) return true; + } catch { + // Not a valid hostname — can't be a metadata IP + } + return false; +} export function validateNavigationUrl(url: string): void { let parsed: URL; @@ -23,10 +57,11 @@ export function validateNavigationUrl(url: string): void { ); } - const hostname = parsed.hostname.toLowerCase(); - if (BLOCKED_METADATA_HOSTS.includes(hostname)) { + const hostname = normalizeHostname(parsed.hostname.toLowerCase()); + + if (BLOCKED_METADATA_HOSTS.has(hostname) || isMetadataIp(hostname)) { throw new Error( - `Blocked: ${hostname} is a cloud metadata endpoint. Access is denied for security.` + `Blocked: ${parsed.hostname} is a cloud metadata endpoint. Access is denied for security.` ); } } diff --git a/browse/test/url-validation.test.ts b/browse/test/url-validation.test.ts index 529ca95d..f87f4e84 100644 --- a/browse/test/url-validation.test.ts +++ b/browse/test/url-validation.test.ts @@ -42,6 +42,26 @@ describe('validateNavigationUrl', () => { expect(() => validateNavigationUrl('http://metadata.google.internal/computeMetadata/v1/')).toThrow(/cloud metadata/i); }); + it('blocks metadata hostname with trailing dot', () => { + expect(() => validateNavigationUrl('http://metadata.google.internal./computeMetadata/v1/')).toThrow(/cloud metadata/i); + }); + + it('blocks metadata IP in hex form', () => { + expect(() => validateNavigationUrl('http://0xA9FEA9FE/')).toThrow(/cloud metadata/i); + }); + + it('blocks metadata IP in decimal form', () => { + expect(() => validateNavigationUrl('http://2852039166/')).toThrow(/cloud metadata/i); + }); + + it('blocks metadata IP in octal form', () => { + expect(() => validateNavigationUrl('http://0251.0376.0251.0376/')).toThrow(/cloud metadata/i); + }); + + it('blocks IPv6 metadata with brackets', () => { + expect(() => validateNavigationUrl('http://[fd00::]/')).toThrow(/cloud metadata/i); + }); + it('throws on malformed URLs', () => { expect(() => validateNavigationUrl('not-a-url')).toThrow(/Invalid URL/i); });