From 5bfd7b24680cbf807b5ea17d83f05edc33c6f5f2 Mon Sep 17 00:00:00 2001 From: Dayuan Jiang <34411969+DayuanJiang@users.noreply.github.com> Date: Sun, 28 Jun 2026 12:41:29 +0900 Subject: [PATCH] fix: SSRF in /api/parse-url via DNS bypass and redirects (#878) * fix: resolve DNS before SSRF check and block redirects in parse-url isPrivateUrl() did string-only hostname matching and never resolved DNS, so a public-looking name that maps to an internal IP (e.g. 127-0-0-1.sslip.io -> 127.0.0.1) passed the check while fetch/extract later resolved it and reached internal services (GHSA-wqcv-5qvx-vx75). - isPrivateUrl is now async: it keeps the fast string/literal-IP path, then resolves the hostname via DNS and rejects if any address is private. - parse-url now fetches the page itself with redirect: "error" and parses via extractFromHtml(), since article-extractor follows redirects internally and drops a redirect option, which allowed a public URL to 302 to an internal host. - Update validate-model call site to await; add regression tests. * fix: preserve charset detection and block CGNAT range in parse-url SSRF fix Follow-up to the multi-reviewer review of the SSRF fix: - Restore charset handling lost when switching from extract() to response.text(): non-UTF-8 pages (Shift_JIS/GBK/EUC/Big5, common on CJK sites this project targets) decoded as mojibake. Now read the body as bytes, detect charset from Content-Type / , and decode with TextDecoder before extractFromHtml. - Wrap extractFromHtml in try/catch: it throws (not returns null) on empty/non-HTML bodies, which previously surfaced as a 500 instead of the intended 400. - Add 100.64.0.0/10 (RFC 6598 CGNAT) to isPrivateIp; it is routable inside some cloud internal networks and was a residual SSRF target. - Add tests for CGNAT, its boundaries, 0.0.0.0, and DNS-resolved IPv6. --- app/api/parse-url/route.ts | 95 ++++++++++++----- app/api/validate-model/route.ts | 2 +- lib/ssrf-protection.ts | 160 +++++++++++++++++------------ tests/unit/ssrf-protection.test.ts | 86 +++++++++++++--- 4 files changed, 234 insertions(+), 109 deletions(-) diff --git a/app/api/parse-url/route.ts b/app/api/parse-url/route.ts index 4dd6612..794b711 100644 --- a/app/api/parse-url/route.ts +++ b/app/api/parse-url/route.ts @@ -1,4 +1,4 @@ -import { extract } from "@extractus/article-extractor" +import { extractFromHtml } from "@extractus/article-extractor" import { NextResponse } from "next/server" import TurndownService from "turndown" import { isPrivateUrl } from "@/lib/ssrf-protection" @@ -7,6 +7,31 @@ const MAX_CONTENT_LENGTH = 150000 // Match PDF limit const EXTRACT_TIMEOUT_MS = 15000 const USER_AGENT = "Mozilla/5.0 (compatible; NextAIDrawio/1.0)" +// Detect the page's charset so non-UTF-8 pages (Shift_JIS/GBK/EUC/Big5, common +// on CJK sites) are decoded correctly. Response.text() always assumes UTF-8 and +// would produce mojibake; the article-extractor library does the same detection +// when it fetches the page itself, which we no longer rely on. +function detectCharset( + contentType: string | null, + buffer: ArrayBuffer, +): string { + // 1. HTTP Content-Type header charset (most authoritative). + const headerCharset = contentType?.match(/charset=([^;]+)/i)?.[1]?.trim() + // 2. / in the first bytes of the document. + const head = new TextDecoder("utf-8").decode(buffer.slice(0, 4096)) + const metaCharset = + head.match(/]+charset=["']?\s*([\w-]+)/i)?.[1] || + head.match(/]+content=["'][^"']*charset=([\w-]+)/i)?.[1] + const charset = (headerCharset || metaCharset || "utf-8").toLowerCase() + // TextDecoder throws on unknown encoding labels; fall back to UTF-8. + try { + new TextDecoder(charset) + return charset + } catch { + return "utf-8" + } +} + export async function POST(req: Request) { try { const { url } = await req.json() @@ -31,21 +56,31 @@ export async function POST(req: Request) { // SSRF protection: parse-url has no use case for fetching internal // hosts, so private URLs are always rejected. ALLOW_PRIVATE_URLS only // governs LLM provider baseUrl overrides (validate-model, chat). - if (isPrivateUrl(url)) { + if (await isPrivateUrl(url)) { return NextResponse.json( { error: "Cannot access private/internal URLs" }, { status: 400 }, ) } - const headController = new AbortController() - const headTimeout = setTimeout(() => headController.abort(), 3000) + // Fetch the page ourselves so we control redirect handling. The + // article-extractor library follows redirects internally and ignores a + // `redirect` option, which would let a public URL 302 to an internal + // host and bypass the SSRF check above. `redirect: "error"` rejects any + // redirect outright. + const controller = new AbortController() + const timeoutId = setTimeout(() => { + controller.abort() + }, EXTRACT_TIMEOUT_MS) + + let html: string try { - const headResponse = await fetch(url, { - method: "HEAD", + const response = await fetch(url, { headers: { "User-Agent": USER_AGENT }, - signal: headController.signal, + redirect: "error", + signal: controller.signal, }) - const contentType = headResponse.headers.get("content-type") + + const contentType = response.headers.get("content-type") if (contentType?.includes("application/pdf")) { return NextResponse.json( { @@ -54,27 +89,17 @@ export async function POST(req: Request) { { status: 422 }, ) } - } catch (err) { - console.warn( - "HEAD pre-check failed, proceeding with extraction:", - err, - ) - } finally { - clearTimeout(headTimeout) - } - // Extract article content with timeout to avoid tying up server resources - const controller = new AbortController() - const timeoutId = setTimeout(() => { - controller.abort() - }, EXTRACT_TIMEOUT_MS) + if (!response.ok) { + return NextResponse.json( + { error: "Could not fetch URL content" }, + { status: 400 }, + ) + } - let article - try { - article = await extract(url, undefined, { - headers: { "User-Agent": USER_AGENT }, - signal: controller.signal, - }) + const buffer = await response.arrayBuffer() + const charset = detectCharset(contentType, buffer) + html = new TextDecoder(charset).decode(buffer) } catch (err: any) { if (err?.name === "AbortError") { return NextResponse.json( @@ -82,11 +107,25 @@ export async function POST(req: Request) { { status: 504 }, ) } - throw err + // Redirects are rejected with a TypeError ("failed to fetch" / + // "unexpected redirect") when redirect: "error" is set. + return NextResponse.json( + { error: "Could not fetch URL content" }, + { status: 400 }, + ) } finally { clearTimeout(timeoutId) } + // extractFromHtml throws (not returns null) on empty/non-HTML bodies, + // so map any parse error to the same 400 as the no-content case. + let article: Awaited> + try { + article = await extractFromHtml(html, url) + } catch { + article = null + } + if (!article || !article.content) { return NextResponse.json( { error: "Could not extract content from URL" }, diff --git a/app/api/validate-model/route.ts b/app/api/validate-model/route.ts index 1b40c48..6f3b5b9 100644 --- a/app/api/validate-model/route.ts +++ b/app/api/validate-model/route.ts @@ -56,7 +56,7 @@ export async function POST(req: Request) { } // SECURITY: Block SSRF attacks via custom baseUrl - if (baseUrl && !allowPrivateUrls() && isPrivateUrl(baseUrl)) { + if (baseUrl && !allowPrivateUrls() && (await isPrivateUrl(baseUrl))) { return NextResponse.json( { valid: false, error: "Invalid base URL" }, { status: 400 }, diff --git a/lib/ssrf-protection.ts b/lib/ssrf-protection.ts index 30593a6..6b43ddf 100644 --- a/lib/ssrf-protection.ts +++ b/lib/ssrf-protection.ts @@ -2,80 +2,108 @@ * SSRF (Server-Side Request Forgery) protection utilities */ +import { lookup } from "node:dns/promises" + /** - * Check if URL points to private/internal network - * Blocks: localhost, private IPs, link-local, AWS metadata service + * Check if an IP address (IPv4 or IPv6) belongs to a private/internal range. + * Works for both user-supplied literal IPs and DNS-resolved addresses. */ -export function isPrivateUrl(urlString: string): boolean { +function isPrivateIp(ip: string): boolean { + const addr = ip.toLowerCase().replace(/^\[|\]$/g, "") + + // IPv6 + if (addr.includes(":")) { + if (addr === "::1" || addr === "::") return true + // unique-local (fc00::/7) and IPv4-mapped (::ffff:0:0/96) + if ( + addr.startsWith("fc") || + addr.startsWith("fd") || + addr.startsWith("::ffff:") + ) { + return true + } + // link-local (fe80::/10) + const linkLocal = addr.match(/^fe([0-9a-f]{2}):/) + if (linkLocal) { + const high = parseInt(linkLocal[1], 16) + if (high >= 0x80 && high <= 0xbf) return true + } + return false + } + + // IPv4 + const ipv4Match = addr.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/) + if (ipv4Match) { + const [, a, b] = ipv4Match.map(Number) + if (a === 10) return true // 10.0.0.0/8 + if (a === 172 && b >= 16 && b <= 31) return true // 172.16.0.0/12 + if (a === 192 && b === 168) return true // 192.168.0.0/16 + if (a === 169 && b === 254) return true // 169.254.0.0/16 (link-local) + if (a === 127) return true // 127.0.0.0/8 (loopback) + if (a === 0) return true // 0.0.0.0/8 + if (a === 100 && b >= 64 && b <= 127) return true // 100.64.0.0/10 (CGNAT, used by some cloud internal networks) + } + + return false +} + +/** + * String-only check against well-known private hostnames and literal IPs. + * Fast path that avoids a DNS lookup for obvious cases. + */ +function isPrivateHostname(hostname: string): boolean { + const host = hostname + .toLowerCase() + .replace(/^\[|\]$/g, "") + .replace(/\.$/, "") + + if ( + host === "localhost" || + host === "127.0.0.1" || + host === "::1" || + host === "::" + ) { + return true + } + + if (host === "169.254.169.254" || host === "metadata.google.internal") { + return true + } + + if ( + host.endsWith(".local") || + host.endsWith(".internal") || + host.endsWith(".localhost") + ) { + return true + } + + // Literal IP supplied directly in the URL + return isPrivateIp(host) +} + +/** + * Check if URL points to private/internal network. + * Blocks: localhost, private IPs, link-local, AWS metadata service. + * + * Resolves the hostname via DNS and validates every returned address, so + * public-looking names that map to internal IPs (e.g. "127-0-0-1.sslip.io") + * are caught even though they pass the string-only check. + */ +export async function isPrivateUrl(urlString: string): Promise { try { const url = new URL(urlString) - // Strip a trailing dot so FQDN forms like "localhost." (which still - // resolve to 127.0.0.1) cannot bypass the equality checks below. const hostname = url.hostname - .toLowerCase() - .replace(/^\[|\]$/g, "") - .replace(/\.$/, "") - // Block localhost - if ( - hostname === "localhost" || - hostname === "127.0.0.1" || - hostname === "::1" || - hostname === "::" - ) { - return true - } + // Fast path: obvious string matches and literal IPs. + if (isPrivateHostname(hostname)) return true - // Block IPv6 unique-local (fc00::/7), link-local (fe80::/10), - // and IPv4-mapped (::ffff:0:0/96) hosts. - if (hostname.includes(":")) { - if ( - hostname.startsWith("fc") || - hostname.startsWith("fd") || - hostname.startsWith("::ffff:") - ) { - return true - } - const linkLocal = hostname.match(/^fe([0-9a-f]{2}):/) - if (linkLocal) { - const high = parseInt(linkLocal[1], 16) - if (high >= 0x80 && high <= 0xbf) return true - } - } - - // Block AWS/cloud metadata endpoints - if ( - hostname === "169.254.169.254" || - hostname === "metadata.google.internal" - ) { - return true - } - - // Check for private IPv4 ranges - const ipv4Match = hostname.match( - /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/, - ) - if (ipv4Match) { - const [, a, b] = ipv4Match.map(Number) - if (a === 10) return true // 10.0.0.0/8 - if (a === 172 && b >= 16 && b <= 31) return true // 172.16.0.0/12 - if (a === 192 && b === 168) return true // 192.168.0.0/16 - if (a === 169 && b === 254) return true // 169.254.0.0/16 (link-local) - if (a === 127) return true // 127.0.0.0/8 (loopback) - } - - // Block common internal hostnames - if ( - hostname.endsWith(".local") || - hostname.endsWith(".internal") || - hostname.endsWith(".localhost") - ) { - return true - } - - return false + // Resolve DNS and reject if any address is private. + const stripped = hostname.replace(/^\[|\]$/g, "").replace(/\.$/, "") + const addresses = await lookup(stripped, { all: true }) + return addresses.some(({ address }) => isPrivateIp(address)) } catch { - return true // Invalid URL - block it + return true // Invalid URL or DNS failure - block it } } diff --git a/tests/unit/ssrf-protection.test.ts b/tests/unit/ssrf-protection.test.ts index 4a405eb..a05eab3 100644 --- a/tests/unit/ssrf-protection.test.ts +++ b/tests/unit/ssrf-protection.test.ts @@ -1,21 +1,79 @@ -import { describe, expect, it } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { isPrivateUrl } from "@/lib/ssrf-protection" +// Mock DNS so tests are deterministic and never hit the network. +const lookupMock = vi.hoisted(() => vi.fn()) +vi.mock("node:dns/promises", () => ({ + default: { lookup: lookupMock }, + lookup: lookupMock, +})) + describe("isPrivateUrl", () => { - it("blocks private IPv6 URLs", () => { - expect(isPrivateUrl("http://[::1]/")).toBe(true) - expect(isPrivateUrl("http://[0:0:0:0:0:0:0:1]/")).toBe(true) - expect(isPrivateUrl("http://[::]/")).toBe(true) - expect(isPrivateUrl("http://[::ffff:127.0.0.1]/")).toBe(true) - expect(isPrivateUrl("http://[fc00::1]/")).toBe(true) - expect(isPrivateUrl("http://[fd12:3456:789a::1]/")).toBe(true) - expect(isPrivateUrl("http://[fe80::1]/")).toBe(true) - expect(isPrivateUrl("http://[fe9f::1]/")).toBe(true) - expect(isPrivateUrl("http://[febf::1]/")).toBe(true) + beforeEach(() => { + lookupMock.mockReset() }) - it("allows public URLs", () => { - expect(isPrivateUrl("https://example.com/article")).toBe(false) - expect(isPrivateUrl("https://fc00.example.com/article")).toBe(false) + it("blocks private IPv6 URLs (string-only fast path, no DNS)", async () => { + expect(await isPrivateUrl("http://[::1]/")).toBe(true) + expect(await isPrivateUrl("http://[0:0:0:0:0:0:0:1]/")).toBe(true) + expect(await isPrivateUrl("http://[::]/")).toBe(true) + expect(await isPrivateUrl("http://[::ffff:127.0.0.1]/")).toBe(true) + expect(await isPrivateUrl("http://[fc00::1]/")).toBe(true) + expect(await isPrivateUrl("http://[fd12:3456:789a::1]/")).toBe(true) + expect(await isPrivateUrl("http://[fe80::1]/")).toBe(true) + expect(await isPrivateUrl("http://[fe9f::1]/")).toBe(true) + expect(await isPrivateUrl("http://[febf::1]/")).toBe(true) + expect(lookupMock).not.toHaveBeenCalled() + }) + + it("blocks literal private IPv4 without DNS", async () => { + expect(await isPrivateUrl("http://127.0.0.1/")).toBe(true) + expect(await isPrivateUrl("http://10.0.0.5/")).toBe(true) + expect(await isPrivateUrl("http://192.168.1.1/")).toBe(true) + expect(await isPrivateUrl("http://169.254.169.254/")).toBe(true) + expect(await isPrivateUrl("http://0.0.0.0/")).toBe(true) + // 100.64.0.0/10 CGNAT (RFC 6598), routable in some cloud internal nets + expect(await isPrivateUrl("http://100.64.0.1/")).toBe(true) + expect(await isPrivateUrl("http://100.127.255.255/")).toBe(true) + expect(lookupMock).not.toHaveBeenCalled() + }) + + it("treats CGNAT boundaries correctly", async () => { + // 100.63.x and 100.128.x are outside 100.64.0.0/10 → public + lookupMock.mockResolvedValue([{ address: "100.63.255.255", family: 4 }]) + expect(await isPrivateUrl("http://just-below.example/")).toBe(false) + lookupMock.mockResolvedValue([{ address: "100.128.0.1", family: 4 }]) + expect(await isPrivateUrl("http://just-above.example/")).toBe(false) + }) + + it("blocks a hostname that resolves to a private IPv6 address", async () => { + lookupMock.mockResolvedValue([{ address: "fd00::1", family: 6 }]) + expect(await isPrivateUrl("http://v6.example.com/")).toBe(true) + }) + + it("allows public URLs that resolve to public IPs", async () => { + lookupMock.mockResolvedValue([{ address: "93.184.216.34", family: 4 }]) + expect(await isPrivateUrl("https://example.com/article")).toBe(false) + }) + + it("blocks public-looking hostnames that resolve to a private IP (DNS-rebinding-style bypass)", async () => { + // e.g. 127-0-0-1.sslip.io resolves to 127.0.0.1 + lookupMock.mockResolvedValue([{ address: "127.0.0.1", family: 4 }]) + expect(await isPrivateUrl("http://127-0-0-1.sslip.io/")).toBe(true) + }) + + it("blocks when any resolved address is private", async () => { + lookupMock.mockResolvedValue([ + { address: "93.184.216.34", family: 4 }, + { address: "10.1.2.3", family: 4 }, + ]) + expect(await isPrivateUrl("http://mixed.example.com/")).toBe(true) + }) + + it("blocks when DNS resolution fails", async () => { + lookupMock.mockRejectedValue(new Error("ENOTFOUND")) + expect(await isPrivateUrl("http://does-not-resolve.example/")).toBe( + true, + ) }) })