security: validate Planner callback URL to close an SSRF sink
The session callbackUrl becomes a server-side fetch target in api.save-to-journal (POSTed with the callback bearer token, and the journal's response is reflected to the caller). The /new query-param loader already validated it, but the programmatic POST /api/sessions entry point — anonymous, since the Planner is stateless — stored it unvalidated. An attacker could make the Planner backend POST to arbitrary hosts, including 169.254.169.254 and other internal targets. - validateFetchUrl now blocks private / loopback / link-local / CGNAT / cloud-metadata hosts (IPv4, IPv6, IPv4-mapped) when an explicit allowlist isn't set. Gated on NODE_ENV=production && !E2E (the requireSecret idiom) so the dev/e2e journal-on-localhost save flow is unaffected. An explicit PLANNER_CALLBACK_ALLOWED_HOSTS still takes precedence and remains the full-closure control (it also stops DNS-name-to-private rebinding, which literal blocking does not). - POST /api/sessions now validates callbackUrl exactly as /new does. - api.save-to-journal re-validates immediately before the fetch (defense in depth: covers sessions persisted before this change and narrows the create→save rebinding window). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
e64155b490
commit
c3454641df
4 changed files with 139 additions and 0 deletions
|
|
@ -44,6 +44,59 @@ describe("validateFetchUrl", () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("validateFetchUrl — private-address blocking (production)", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.unstubAllEnvs();
|
||||||
|
// Production-without-E2E is the only mode that blocks (mirrors the
|
||||||
|
// requireSecret guard). Tests otherwise run as NODE_ENV=test → off.
|
||||||
|
vi.stubEnv("NODE_ENV", "production");
|
||||||
|
vi.stubEnv("E2E", "");
|
||||||
|
});
|
||||||
|
|
||||||
|
const blocked = [
|
||||||
|
"http://127.0.0.1/x",
|
||||||
|
"http://localhost:3000/x",
|
||||||
|
"http://sub.localhost/x",
|
||||||
|
"http://169.254.169.254/latest/meta-data/", // cloud metadata
|
||||||
|
"http://10.0.0.5/x",
|
||||||
|
"http://172.16.0.1/x",
|
||||||
|
"http://172.31.255.254/x",
|
||||||
|
"http://192.168.1.1/x",
|
||||||
|
"http://100.64.0.1/x", // CGNAT
|
||||||
|
"http://0.0.0.0/x",
|
||||||
|
"http://[::1]/x",
|
||||||
|
"http://[fc00::1]/x",
|
||||||
|
"http://[fe80::1]/x",
|
||||||
|
"http://[::ffff:127.0.0.1]/x",
|
||||||
|
];
|
||||||
|
for (const url of blocked) {
|
||||||
|
it(`blocks ${url}`, () => {
|
||||||
|
expect(validateFetchUrl(url).ok).toBe(false);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
it("still allows public hosts", () => {
|
||||||
|
expect(validateFetchUrl("https://journal.trails.cool/api/cb").ok).toBe(true);
|
||||||
|
expect(validateFetchUrl("http://203.0.113.10/x").ok).toBe(true); // public IP literal
|
||||||
|
expect(validateFetchUrl("http://172.15.0.1/x").ok).toBe(true); // just outside RFC1918 /12
|
||||||
|
expect(validateFetchUrl("http://172.32.0.1/x").ok).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("an explicit allowlist overrides private blocking (operator decision)", () => {
|
||||||
|
expect(
|
||||||
|
validateFetchUrl("http://10.0.0.2:3000/cb", { allowedHosts: ["10.0.0.2:3000"] }).ok,
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not block private hosts outside production (dev/e2e localhost flow)", () => {
|
||||||
|
vi.stubEnv("NODE_ENV", "development");
|
||||||
|
expect(validateFetchUrl("http://localhost:3000/cb").ok).toBe(true);
|
||||||
|
vi.stubEnv("NODE_ENV", "production");
|
||||||
|
vi.stubEnv("E2E", "true");
|
||||||
|
expect(validateFetchUrl("http://localhost:3000/cb").ok).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("validateRedirectUrl", () => {
|
describe("validateRedirectUrl", () => {
|
||||||
it("accepts an absolute https URL", () => {
|
it("accepts an absolute https URL", () => {
|
||||||
expect(validateRedirectUrl("https://trails.cool/r/123").ok).toBe(true);
|
expect(validateRedirectUrl("https://trails.cool/r/123").ok).toBe(true);
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,60 @@ export interface UrlValidationResult {
|
||||||
url?: URL;
|
url?: URL;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether to reject callback hosts pointing at private / loopback /
|
||||||
|
* link-local / cloud-metadata ranges. Active in real production only —
|
||||||
|
* mirrors the `requireSecret` guard in the journal's config.server.ts.
|
||||||
|
* Dev and E2E legitimately point the callback at the journal on
|
||||||
|
* `localhost`, so blocking there would break the save flow.
|
||||||
|
*/
|
||||||
|
function blockPrivateAddresses(): boolean {
|
||||||
|
return process.env.NODE_ENV === "production" && process.env.E2E !== "true";
|
||||||
|
}
|
||||||
|
|
||||||
|
function isBlockedIpv4(ip: string): boolean {
|
||||||
|
const o = ip.split(".").map((n) => Number(n));
|
||||||
|
// Malformed dotted-quad → treat as blocked (fail safe).
|
||||||
|
if (o.length !== 4 || o.some((n) => !Number.isInteger(n) || n < 0 || n > 255)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
const [a, b] = o as [number, number, number, number];
|
||||||
|
if (a === 0 || a === 10 || a === 127) return true; // this-host, RFC1918 /8, loopback
|
||||||
|
if (a === 169 && b === 254) return true; // link-local incl. 169.254.169.254 metadata
|
||||||
|
if (a === 172 && b >= 16 && b <= 31) return true; // RFC1918 /12
|
||||||
|
if (a === 192 && b === 168) return true; // RFC1918 /16
|
||||||
|
if (a === 100 && b >= 64 && b <= 127) return true; // CGNAT 100.64/10
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Literal private/loopback/link-local hosts. Catches IP literals and
|
||||||
|
* `localhost`; it does NOT resolve DNS, so a public name that resolves
|
||||||
|
* to a private IP (DNS-rebinding-style SSRF) is not blocked here — set
|
||||||
|
* PLANNER_CALLBACK_ALLOWED_HOSTS to close that where the callback host
|
||||||
|
* is known ahead of time.
|
||||||
|
*/
|
||||||
|
function isBlockedHost(hostname: string): boolean {
|
||||||
|
// Node keeps IPv6 brackets on URL.hostname (e.g. "[::1]"); strip them.
|
||||||
|
const h = hostname.toLowerCase().replace(/^\[|\]$/g, "");
|
||||||
|
if (h === "localhost" || h.endsWith(".localhost")) return true;
|
||||||
|
if (h.includes(":")) {
|
||||||
|
// IPv6.
|
||||||
|
if (h === "::1" || h === "::") return true;
|
||||||
|
if (h.startsWith("fc") || h.startsWith("fd")) return true; // ULA fc00::/7
|
||||||
|
if (h.startsWith("fe8") || h.startsWith("fe9") || h.startsWith("fea") || h.startsWith("feb")) {
|
||||||
|
return true; // link-local fe80::/10
|
||||||
|
}
|
||||||
|
// IPv4-mapped (::ffff:a.b.c.d, which Node may render in hex form).
|
||||||
|
// Reaching IPv4 through a mapped address is inherently suspect for a
|
||||||
|
// callback target, so block the whole prefix.
|
||||||
|
if (h.startsWith("::ffff:")) return true;
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(h)) return isBlockedIpv4(h);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Validate a URL string for use as a fetch target. Requires absolute
|
* Validate a URL string for use as a fetch target. Requires absolute
|
||||||
* http(s) URL. If `allowedHosts` is provided, the host must be on the
|
* http(s) URL. If `allowedHosts` is provided, the host must be on the
|
||||||
|
|
@ -39,9 +93,16 @@ export function validateFetchUrl(
|
||||||
return { ok: false, reason: `disallowed scheme ${parsed.protocol}` };
|
return { ok: false, reason: `disallowed scheme ${parsed.protocol}` };
|
||||||
}
|
}
|
||||||
if (opts.allowedHosts && opts.allowedHosts.length > 0) {
|
if (opts.allowedHosts && opts.allowedHosts.length > 0) {
|
||||||
|
// An explicit allowlist is the operator's decision and the strongest
|
||||||
|
// control; a host that matches it is trusted even if it's private.
|
||||||
if (!opts.allowedHosts.includes(parsed.host)) {
|
if (!opts.allowedHosts.includes(parsed.host)) {
|
||||||
return { ok: false, reason: `host ${parsed.host} not on allowlist` };
|
return { ok: false, reason: `host ${parsed.host} not on allowlist` };
|
||||||
}
|
}
|
||||||
|
} else if (blockPrivateAddresses() && isBlockedHost(parsed.hostname)) {
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
reason: `host ${parsed.hostname} is a private/loopback/link-local address`,
|
||||||
|
};
|
||||||
}
|
}
|
||||||
return { ok: true, url: parsed };
|
return { ok: true, url: parsed };
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,7 @@ import { data } from "react-router";
|
||||||
import type { Route } from "./+types/api.save-to-journal";
|
import type { Route } from "./+types/api.save-to-journal";
|
||||||
import { getSession } from "~/lib/sessions";
|
import { getSession } from "~/lib/sessions";
|
||||||
import { fetchWithTimeout } from "~/lib/http.server";
|
import { fetchWithTimeout } from "~/lib/http.server";
|
||||||
|
import { validateFetchUrl, getCallbackAllowedHosts } from "~/lib/url-validation.server";
|
||||||
|
|
||||||
interface SaveRequestBody {
|
interface SaveRequestBody {
|
||||||
sessionId?: unknown;
|
sessionId?: unknown;
|
||||||
|
|
@ -51,6 +52,15 @@ export async function action({ request }: Route.ActionArgs) {
|
||||||
return data({ error: "session has no journal callback" }, { status: 400 });
|
return data({ error: "session has no journal callback" }, { status: 400 });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Defense in depth: re-validate immediately before the outbound fetch.
|
||||||
|
// Guards sessions persisted before callbackUrl validation existed, and
|
||||||
|
// narrows the window for a host that was public at create time but
|
||||||
|
// resolves private now.
|
||||||
|
const v = validateFetchUrl(session.callbackUrl, { allowedHosts: getCallbackAllowedHosts() });
|
||||||
|
if (!v.ok) {
|
||||||
|
return data({ error: "session callback URL is not allowed" }, { status: 400 });
|
||||||
|
}
|
||||||
|
|
||||||
let resp: Response;
|
let resp: Response;
|
||||||
try {
|
try {
|
||||||
resp = await fetchWithTimeout(session.callbackUrl, {
|
resp = await fetchWithTimeout(session.callbackUrl, {
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,7 @@ import { createSession, listSessions } from "~/lib/sessions";
|
||||||
import { parseGpxAsync, extractWaypoints } from "@trails-cool/gpx";
|
import { parseGpxAsync, extractWaypoints } from "@trails-cool/gpx";
|
||||||
import { withDb } from "@trails-cool/db";
|
import { withDb } from "@trails-cool/db";
|
||||||
import type { Waypoint } from "@trails-cool/types";
|
import type { Waypoint } from "@trails-cool/types";
|
||||||
|
import { validateFetchUrl, getCallbackAllowedHosts } from "~/lib/url-validation.server";
|
||||||
|
|
||||||
export async function action({ request }: Route.ActionArgs) {
|
export async function action({ request }: Route.ActionArgs) {
|
||||||
if (request.method !== "POST") {
|
if (request.method !== "POST") {
|
||||||
|
|
@ -17,6 +18,20 @@ export async function action({ request }: Route.ActionArgs) {
|
||||||
gpx?: string;
|
gpx?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// callbackUrl becomes a server-side fetch target on save-to-journal,
|
||||||
|
// so an unvalidated value here is an SSRF sink. The /new loader
|
||||||
|
// already validates the query-param form; this is the programmatic
|
||||||
|
// JSON entry point and must do the same.
|
||||||
|
if (callbackUrl !== undefined) {
|
||||||
|
if (typeof callbackUrl !== "string") {
|
||||||
|
return data({ error: "callbackUrl must be a string" }, { status: 400 });
|
||||||
|
}
|
||||||
|
const v = validateFetchUrl(callbackUrl, { allowedHosts: getCallbackAllowedHosts() });
|
||||||
|
if (!v.ok) {
|
||||||
|
return data({ error: `Invalid callback URL: ${v.reason}` }, { status: 400 });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return withDb(async () => {
|
return withDb(async () => {
|
||||||
const session = await createSession({ callbackUrl, callbackToken });
|
const session = await createSession({ callbackUrl, callbackToken });
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue