mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(egress): check every redirect hop and close the reserved-range gaps
safeFetch with requirePublicHost vetted the first URL and pinned DNS answers in the connector, but Node never looks up an IP literal, so a followed redirect to a literal private or metadata address was dialled unchecked. No caller follows redirects to a user-chosen host today; the floor did not hold for the next one that would. With requirePublicHost, redirects are now followed one hop at a time and every location goes through isPublicUrl before it is dialled, at most five hops. isPublicIp also refuses the benchmarking, protocol-assignment, multicast, reserved and broadcast IPv4 ranges, site-local and multicast IPv6 and the IPv4-translated spelling of a private address, and isPublicUrl strips a trailing dot before the hostname denylist.
This commit is contained in:
1 parent
e021adde40
commit
807d483b89
4 files changed
+174
No files matched your search
@@ -73,6 +73,16 @@ function mockLookupTo(addresses: dns.LookupAddress[]): void {
|
||||
beforeAll(async () => {
|
||||
server = http.createServer((req, res) => {
|
||||
requestedPaths.push(req.url ?? "");
|
||||
if (req.url === "/redirect-literal") {
|
||||
res.writeHead(302, { location: `http://127.0.0.1:${port}/secret` });
|
||||
res.end();
|
||||
return;
|
||||
}
|
||||
if (req.url === "/redirect-public") {
|
||||
res.writeHead(302, { location: `http://pinned.test:${port}/api` });
|
||||
res.end();
|
||||
return;
|
||||
}
|
||||
if (req.url === "/redirect") {
|
||||
res.writeHead(302, {
|
||||
location: `http://redirect-target.test:${port}/api`,
|
||||
@@ -192,6 +202,38 @@ describe("safeFetch pinned path — real transport", () => {
|
||||
expect(requestedPaths).toEqual(["/redirect"]);
|
||||
}, 20_000);
|
||||
|
||||
it("checks a followed redirect to an IP literal, which the DNS pin never sees", async () => {
|
||||
mockLookupTo([{ address: "127.0.0.1", family: 4 }]);
|
||||
// The input-time floor, as it really decides for a literal: the harness
|
||||
// host passes, the loopback literal the redirect names does not.
|
||||
notificationMocks.isPublicUrl.mockImplementation(
|
||||
(url: string) => !new URL(url).hostname.startsWith("127."),
|
||||
);
|
||||
requestedPaths.length = 0;
|
||||
|
||||
await expect(
|
||||
safeFetch(
|
||||
`http://pinned.test:${port}/redirect-literal`,
|
||||
{},
|
||||
{ requirePublicHost: true, followRedirects: true, timeoutMs: 5000 },
|
||||
),
|
||||
).rejects.toMatchObject({ kind: "private_host" });
|
||||
expect(requestedPaths).toEqual(["/redirect-literal"]);
|
||||
notificationMocks.isPublicUrl.mockImplementation(() => true);
|
||||
}, 20_000);
|
||||
|
||||
it("follows a redirect to a public host, one checked hop at a time", async () => {
|
||||
mockLookupTo([{ address: "127.0.0.1", family: 4 }]);
|
||||
requestedPaths.length = 0;
|
||||
const res = await safeFetch(
|
||||
`http://pinned.test:${port}/redirect-public`,
|
||||
{},
|
||||
{ requirePublicHost: true, followRedirects: true, timeoutMs: 5000 },
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
expect(requestedPaths).toEqual(["/redirect-public", "/api"]);
|
||||
}, 20_000);
|
||||
|
||||
it("streams a chunked response body via getReader() on the pinned path", async () => {
|
||||
mockLookupTo([{ address: "127.0.0.1", family: 4 }]);
|
||||
|
||||
|
||||
@@ -99,6 +99,64 @@ function hasPrivateHostPolicyCause(error: unknown): boolean {
|
||||
);
|
||||
}
|
||||
|
||||
/** Hops a followed redirect chain may take before it is refused. */
|
||||
const MAX_REDIRECTS = 5;
|
||||
|
||||
/**
|
||||
* Follow redirects one hop at a time, putting every new location through
|
||||
* `isPublicUrl` before it is dialled.
|
||||
*
|
||||
* The pinned dispatcher vets an address in its DNS lookup, and Node never
|
||||
* looks up an IP literal, so a redirect to `http://169.254.169.254/` would
|
||||
* have been dialled without a check: the input-time `isPublicUrl` had only
|
||||
* seen the first URL. A 301, 302 or 303 continues as a GET without a body,
|
||||
* as fetch does; a 307 or 308 repeats the request only when it has no body
|
||||
* to send again.
|
||||
*/
|
||||
async function followCheckingEachHop(
|
||||
start: string,
|
||||
init: RequestInit,
|
||||
dispatcher: NonNullable<Parameters<typeof undiciFetch>[1]>["dispatcher"],
|
||||
): Promise<Response> {
|
||||
let url = start;
|
||||
let current: RequestInit = { ...init, redirect: "manual" };
|
||||
for (let hop = 0; ; hop++) {
|
||||
const res = (await undiciFetch(url, {
|
||||
...current,
|
||||
dispatcher,
|
||||
} as Parameters<typeof undiciFetch>[1])) as unknown as Response;
|
||||
const location = res.headers.get("location");
|
||||
if (res.status < 300 || res.status >= 400 || !location) return res;
|
||||
void res.body?.cancel().catch(() => {});
|
||||
if (hop >= MAX_REDIRECTS) {
|
||||
throw new SafeFetchError(
|
||||
"safeFetch followed too many redirects",
|
||||
"network",
|
||||
);
|
||||
}
|
||||
const next = new URL(location, url).toString();
|
||||
if (!isPublicUrl(next)) {
|
||||
throw new SafeFetchError(
|
||||
"safeFetch refused a redirect to a private or non-public host",
|
||||
"private_host",
|
||||
);
|
||||
}
|
||||
if (res.status === 307 || res.status === 308) {
|
||||
if (current.body != null) {
|
||||
throw new SafeFetchError(
|
||||
"safeFetch cannot repeat a request body across a redirect",
|
||||
"network",
|
||||
);
|
||||
}
|
||||
} else {
|
||||
const { body: _body, ...rest } = current;
|
||||
void _body;
|
||||
current = { ...rest, method: "GET" };
|
||||
}
|
||||
url = next;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Default-safe outbound fetch wrapper.
|
||||
*
|
||||
@@ -217,6 +275,9 @@ export async function safeFetch(
|
||||
// locks the dispatcher and the fetch engine to one undici version.
|
||||
// The cast bridges undici's RequestInit (which carries `dispatcher`)
|
||||
// and its Response back to the DOM types this module exposes.
|
||||
if (opts.requirePublicHost && redirect === "follow") {
|
||||
return await followCheckingEachHop(target, finalInit, dispatcher);
|
||||
}
|
||||
const res = await undiciFetch(target, {
|
||||
...finalInit,
|
||||
dispatcher,
|
||||
@@ -225,6 +286,7 @@ export async function safeFetch(
|
||||
}
|
||||
return await fetch(target, finalInit);
|
||||
} catch (err) {
|
||||
if (err instanceof SafeFetchError) throw err;
|
||||
if (hasPrivateHostPolicyCause(err)) {
|
||||
throw new SafeFetchError(
|
||||
"safeFetch refused a private resolved destination",
|
||||
|
||||
@@ -258,3 +258,55 @@ describe("webPushSubscriptionSchema SSRF guard (V3 audit)", () => {
|
||||
expect(r.success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Ranges the floor used to call public: benchmarking, IETF protocol
|
||||
* assignments, multicast, reserved and broadcast IPv4; site-local and
|
||||
* multicast IPv6; the IPv4-translated spelling of a private address; and a
|
||||
* denylisted name written fully qualified with a trailing dot.
|
||||
*/
|
||||
describe("isPublicIp / isPublicUrl — reserved ranges and spellings", () => {
|
||||
it("refuses the reserved IPv4 ranges", () => {
|
||||
for (const ip of [
|
||||
"198.18.0.1",
|
||||
"198.19.255.254",
|
||||
"192.0.0.1",
|
||||
"224.0.0.1",
|
||||
"239.255.255.250",
|
||||
"240.0.0.1",
|
||||
"255.255.255.255",
|
||||
]) {
|
||||
expect(isPublicIp(ip), ip).toBe(false);
|
||||
}
|
||||
for (const ip of ["198.17.0.1", "198.20.0.1", "192.0.1.1", "223.255.0.1"]) {
|
||||
expect(isPublicIp(ip), ip).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses site-local, multicast and IPv4-translated IPv6", () => {
|
||||
for (const ip of [
|
||||
"fec0::1",
|
||||
"feff::1",
|
||||
"ff02::1",
|
||||
"::ffff:0:7f00:1",
|
||||
"::ffff:0:a9fe:a9fe",
|
||||
]) {
|
||||
expect(isPublicIp(ip), ip).toBe(false);
|
||||
}
|
||||
expect(isPublicIp("::ffff:0:808:808")).toBe(true);
|
||||
});
|
||||
|
||||
it("refuses a denylisted name written with a trailing dot", () => {
|
||||
for (const url of [
|
||||
"http://localhost./",
|
||||
"http://localhost../x",
|
||||
"http://metadata.google.internal./",
|
||||
"http://printer.local./",
|
||||
"http://127.0.0.1./",
|
||||
"http://[::ffff:0:7f00:1]/",
|
||||
]) {
|
||||
expect(isPublicUrl(url), url).toBe(false);
|
||||
}
|
||||
expect(isPublicUrl("https://example.com./")).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -75,6 +75,9 @@ function isPrivateIpv4(ip: [number, number, number, number]): boolean {
|
||||
if (a === 172 && b >= 16 && b <= 31) return true; // 172.16.0.0/12 RFC1918
|
||||
if (a === 192 && b === 168) return true; // 192.168.0.0/16 RFC1918
|
||||
if (a === 100 && b >= 64 && b <= 127) return true; // 100.64.0.0/10 CGNAT
|
||||
if (a === 192 && b === 0 && ip[2] === 0) return true; // 192.0.0.0/24 IETF protocol assignments
|
||||
if (a === 198 && (b === 18 || b === 19)) return true; // 198.18.0.0/15 benchmarking
|
||||
if (a >= 224) return true; // 224/4 multicast, 240/4 reserved, broadcast
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -155,6 +158,14 @@ function embeddedIpv4(
|
||||
return [bytes[12], bytes[13], bytes[14], bytes[15]];
|
||||
}
|
||||
|
||||
// IPv4-translated (`::ffff:0:a.b.c.d`, RFC 2765).
|
||||
if (
|
||||
bytesEqual(bytes, 0, Array(8).fill(0)) &&
|
||||
bytesEqual(bytes, 8, [0xff, 0xff, 0x00, 0x00])
|
||||
) {
|
||||
return [bytes[12], bytes[13], bytes[14], bytes[15]];
|
||||
}
|
||||
|
||||
if (bytesEqual(bytes, 0, Array(12).fill(0))) {
|
||||
return [bytes[12], bytes[13], bytes[14], bytes[15]];
|
||||
}
|
||||
@@ -219,6 +230,10 @@ export function isPublicIp(ip: string): boolean {
|
||||
return false;
|
||||
}
|
||||
if (bytes[0] === 0xfe && (bytes[1] & 0xc0) === 0x80) return false;
|
||||
// Site-local (fec0::/10, deprecated but still routed inside some
|
||||
// networks) and multicast (ff00::/8).
|
||||
if (bytes[0] === 0xfe && (bytes[1] & 0xc0) === 0xc0) return false;
|
||||
if (bytes[0] === 0xff) return false;
|
||||
if ((bytes[0] & 0xfe) === 0xfc) return false;
|
||||
|
||||
const embedded = embeddedIpv4(bytes);
|
||||
@@ -320,6 +335,9 @@ export function isPublicUrl(url: string): boolean {
|
||||
}
|
||||
let h = parsed.hostname.toLowerCase();
|
||||
if (h.startsWith("[") && h.endsWith("]")) h = h.slice(1, -1);
|
||||
// A fully qualified name ends in a dot (`localhost.`, `foo.internal.`);
|
||||
// the resolver treats it as the same name, so the denylist must too.
|
||||
h = h.replace(/\.+$/, "");
|
||||
|
||||
// Hostname denylist (literal strings).
|
||||
if (
|
||||
|
||||
Reference in new issue
Block a user