diff --git a/changelog.d/fixes/11319-upstream-proxy-host-spelling.md b/changelog.d/fixes/11319-upstream-proxy-host-spelling.md new file mode 100644 index 0000000000..a16ec17bba --- /dev/null +++ b/changelog.d/fixes/11319-upstream-proxy-host-spelling.md @@ -0,0 +1 @@ +- **fix(db):** the upstream proxy URL check judges the host by address instead of by spelling, so `http://[::ffff:169.254.169.254]`, `[::ffff:10.0.0.5]`, ULA/link-local and CGNAT targets are refused like their dotted equivalents ([#11319](https://github.com/diegosouzapw/OmniRoute/pull/11319)) diff --git a/src/lib/db/upstreamProxy.ts b/src/lib/db/upstreamProxy.ts index ea669720b0..ae7ed9a8e1 100644 --- a/src/lib/db/upstreamProxy.ts +++ b/src/lib/db/upstreamProxy.ts @@ -1,5 +1,11 @@ /** Upstream proxy config persistence for upstream_proxy_config table. */ import { getDbInstance } from "./core"; +import { + isCloudMetadataHost, + isPrivateHost as isPrivateNetworkHost, + mappedIpv4Host, +} from "@/shared/network/outboundUrlGuard"; +import { ipVersion, normalizeHost } from "@/shared/network/privateHost"; /** Which embedded proxy handles the retry leg when mode === "fallback". */ export type FallbackBackend = "cliproxyapi" | "dario"; @@ -37,26 +43,39 @@ function toRecord(value: unknown): Record { return value && typeof value === "object" ? (value as Record) : {}; } -const BLOCKED_HOSTNAMES = ["metadata.google.internal", "169.254.169.254", "metadata.aws.internal"]; +const LOOPBACK_HOSTNAMES = new Set(["localhost", "127.0.0.1", "::1"]); +/** IPv4 multicast (224.0.0.0/4) — kept from this module's original rule set. */ +function isMulticastIpv4(host: string): boolean { + const first = Number.parseInt(host.split(".")[0], 10); + return ipVersion(host) === 4 && first >= 224 && first <= 239; +} + +/** + * Reject a proxy target that is private or cloud-metadata, judging the ADDRESS + * rather than its spelling. + * + * This module used to carry its own prefix regexes, which matched only the + * dotted form: `http://169.254.169.254` was refused while + * `http://[::ffff:169.254.169.254]` — the same address, serialised by WHATWG + * URL as `::ffff:a9fe:a9fe` — was accepted, as were `::ffff:10.0.0.5`, + * `fd00::/8`, `fe80::/10` and CGNAT `100.64.0.0/10`. #10843 fixed exactly that + * class in the shared guard; routing this copy through the same helpers keeps + * the two from drifting apart again. + * + * The deliberate exception stays: CLIProxyAPI runs on localhost:8317, so + * loopback is allowed — and now so is its mapped spelling, for the same + * address-not-spelling reason. + */ function isPrivateHost(hostname: string): boolean { - // CLIProxyAPI runs on localhost:8317 — allow loopback explicitly - if (hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1") return false; - if (BLOCKED_HOSTNAMES.includes(hostname)) return true; - if ( - /^10\./.test(hostname) || - /^172\.(1[6-9]|2\d|3[01])\./.test(hostname) || - /^192\.168\./.test(hostname) - ) - return true; - if ( - /^0\./.test(hostname) || - /^127\./.test(hostname) || - /^224\./.test(hostname) || - /^169\.254\./.test(hostname) - ) - return true; - return false; + const normalized = normalizeHost(hostname); + const asIpv4 = mappedIpv4Host(normalized) ?? normalized; + + if (LOOPBACK_HOSTNAMES.has(normalized) || LOOPBACK_HOSTNAMES.has(asIpv4)) return false; + + return ( + isCloudMetadataHost(normalized) || isPrivateNetworkHost(normalized) || isMulticastIpv4(asIpv4) + ); } export function validateProxyUrl( diff --git a/src/shared/network/outboundUrlGuard.ts b/src/shared/network/outboundUrlGuard.ts index 45a7ebde7b..802036b152 100644 --- a/src/shared/network/outboundUrlGuard.ts +++ b/src/shared/network/outboundUrlGuard.ts @@ -39,7 +39,7 @@ export class OutboundUrlGuardError extends Error { // `http://[::ffff:169.254.169.254]/` reaches these helpers as `::ffff:a9fe:a9fe`. // Matching the dotted spelling alone therefore misses every mapped address that // arrives through a parsed URL. Fold the embedded IPv4 back out before deciding. -function mappedIpv4Host(hostname: string): string | null { +export function mappedIpv4Host(hostname: string): string | null { const normalized = normalizeHost(hostname); if (!normalized.startsWith("::ffff:")) return null; const embedded = normalized.slice("::ffff:".length); diff --git a/tests/unit/upstream-proxy-host-spelling.test.ts b/tests/unit/upstream-proxy-host-spelling.test.ts new file mode 100644 index 0000000000..f1696438da --- /dev/null +++ b/tests/unit/upstream-proxy-host-spelling.test.ts @@ -0,0 +1,115 @@ +// `validateProxyUrl()` refused a private/metadata proxy target by matching +// dotted-quad prefixes, so the same address in another spelling walked through. +// Measured on release/v3.8.50 (ac02c5b42): +// +// http://169.254.169.254 -> blocked +// http://[::ffff:169.254.169.254] -> ALLOWED (same address, mapped) +// http://[::ffff:a9fe:a9fe] -> ALLOWED (how WHATWG URL serialises it) +// http://[::ffff:10.0.0.5] -> ALLOWED +// http://[fd00::1] -> ALLOWED (ULA) +// http://[fe80::1] -> ALLOWED (link-local) +// http://100.64.0.1 -> ALLOWED (CGNAT) +// +// #10843 fixed this class in the shared outbound guard; this module kept a +// private copy of the classification and did not get the fix. +import test from "node:test"; +import assert from "node:assert/strict"; + +import { validateProxyUrl } from "../../src/lib/db/upstreamProxy.ts"; + +function isValid(url: string): boolean { + return validateProxyUrl(url).valid; +} + +test("a mapped-IPv4 spelling of a blocked address is blocked too", () => { + for (const url of [ + "http://[::ffff:169.254.169.254]", // cloud metadata, mapped + "http://[::ffff:a9fe:a9fe]", // the same, as WHATWG URL serialises it + "http://[::ffff:10.0.0.5]", // RFC1918, mapped + "http://[::ffff:192.168.1.1]", + "http://[::ffff:172.16.0.1]", + ]) { + assert.equal(isValid(url), false, `${url} must be refused`); + } +}); + +test("private IPv6 ranges are blocked", () => { + for (const url of ["http://[fd00::1]", "http://[fc00::1]", "http://[fe80::1]"]) { + assert.equal(isValid(url), false, `${url} must be refused`); + } +}); + +test("CGNAT space is blocked", () => { + // 100.64.0.0/10 is carrier-grade NAT, not public address space. + assert.equal(isValid("http://100.64.0.1"), false); + assert.equal(isValid("http://100.127.255.254"), false); + // …but the neighbouring public /8 addresses are not. + assert.equal(isValid("http://100.63.255.255"), true); + assert.equal(isValid("http://100.128.0.1"), true); +}); + +test("every address the dotted rules already refused is still refused", () => { + for (const url of [ + "http://169.254.169.254", + "http://metadata.google.internal", + "http://metadata.aws.internal", + "http://10.0.0.5", + "http://172.16.0.1", + "http://172.31.255.255", + "http://192.168.1.1", + "http://0.0.0.0", + "http://127.0.0.2", + "http://224.0.0.1", // IPv4 multicast, the only octet the old rule covered + ]) { + assert.equal(isValid(url), false, `${url} must still be refused`); + } +}); + +test("multicast is refused across the whole /4, not just 224/8", () => { + // Widened on purpose, and the one deliberate behaviour change here beyond + // the spelling fix: the old rule was `/^224\./`, so 225–239 were accepted. + // None of 224.0.0.0/4 can be a proxy. + for (const url of ["http://224.0.0.1", "http://231.7.7.7", "http://239.255.255.250"]) { + assert.equal(isValid(url), false, `${url} must be refused`); + } + assert.equal(isValid("http://240.0.0.1"), true, "just outside the /4 is unchanged"); +}); + +test("loopback stays allowed — CLIProxyAPI runs on localhost:8317", () => { + for (const url of [ + "http://localhost:8317", + "http://127.0.0.1:8317", + "http://[::1]:8317", + // Judging the address rather than its spelling cuts both ways: the mapped + // form of 127.0.0.1 is the same host the exception exists for. + "http://[::ffff:127.0.0.1]:8317", + ]) { + assert.equal(isValid(url), true, `${url} must stay allowed`); + } +}); + +test("ordinary public proxies stay allowed", () => { + for (const url of [ + "http://proxy.example.com", + "https://proxy.example.com:3128", + "http://8.8.8.8:3128", + "http://[2606:4700::1111]", + "http://172.32.0.1", // just outside 172.16.0.0/12 + "http://192.169.0.1", // just outside 192.168.0.0/16 + ]) { + assert.equal(isValid(url), true, `${url} must stay allowed`); + } +}); + +test("the non-host validations are unchanged", () => { + assert.deepEqual(validateProxyUrl("https://proxy.example.com"), { + valid: true, + url: "https://proxy.example.com", + }); + assert.equal(validateProxyUrl("ftp://proxy.example.com").valid, false); + assert.match(String(validateProxyUrl("not-a-url").error), /Invalid URL/); + assert.match( + String(validateProxyUrl("http://169.254.169.254").error), + /private\/internal address/ + ); +});