mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-11 09:22:48 +03:00
Compare commits
1 Commits
fix/12579-
...
fix/12569-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
b6cc3e57c6 |
1
changelog.d/fixes/12569-webhook-dns-rebinding-ssrf.md
Normal file
1
changelog.d/fixes/12569-webhook-dns-rebinding-ssrf.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(api): close DNS-rebinding SSRF gap in webhook outbound-URL guard (#12569)
|
||||
@@ -12,8 +12,7 @@ import { buildTelegramUrl, buildTelegramPayload } from "@/lib/webhooks/integrati
|
||||
import { buildDiscordPayload } from "@/lib/webhooks/integrations/discord";
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { insertDelivery } from "@/lib/db/webhookDeliveries";
|
||||
import { isPrivateHost, OutboundUrlGuardError } from "@/shared/network/outboundUrlGuard";
|
||||
import { parseAndValidateWebhookUrl } from "@/shared/network/outboundUrlGuardPolicy";
|
||||
import { fetchWebhookUrl } from "@/shared/network/webhookFetch";
|
||||
import crypto from "crypto";
|
||||
|
||||
const MAX_RESPONSE_BODY = 2048;
|
||||
@@ -31,35 +30,43 @@ async function testFetch(
|
||||
}> {
|
||||
const start = Date.now();
|
||||
try {
|
||||
const parsed = parseAndValidateWebhookUrl(url);
|
||||
// For private (opted-in) targets, return connectivity diagnostics only — never the
|
||||
// upstream response body, so this endpoint can't be used to exfiltrate content from
|
||||
// internal services reachable from the server. (#3269 hardening)
|
||||
const redactBody = isPrivateHost(parsed.hostname);
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), 10_000);
|
||||
const res = await fetch(url, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Content-Type": "application/json",
|
||||
"User-Agent": "OmniRoute-Webhook/1.0",
|
||||
...headers,
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
signal: controller.signal,
|
||||
});
|
||||
clearTimeout(timeoutId);
|
||||
let response: Response;
|
||||
let redactBody: boolean;
|
||||
try {
|
||||
({ response, redactBody } = await fetchWebhookUrl(
|
||||
url,
|
||||
{
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Content-Type": "application/json",
|
||||
"User-Agent": "OmniRoute-Webhook/1.0",
|
||||
...headers,
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
},
|
||||
{ signal: controller.signal }
|
||||
));
|
||||
} finally {
|
||||
clearTimeout(timeoutId);
|
||||
}
|
||||
const latencyMs = Date.now() - start;
|
||||
// For private (opted-in) targets, return connectivity diagnostics only — never the
|
||||
// upstream response body, so this endpoint can't be used to exfiltrate content from
|
||||
// internal services reachable from the server. (#3269 hardening) The verdict is derived
|
||||
// from the DNS-resolved address, not the raw hostname string, so a public-looking hostname
|
||||
// rebound to a private IP is redacted too.
|
||||
let rawBody = "";
|
||||
try {
|
||||
rawBody = await res.text();
|
||||
rawBody = await response.text();
|
||||
if (rawBody.length > MAX_RESPONSE_BODY) rawBody = rawBody.slice(0, MAX_RESPONSE_BODY) + "…";
|
||||
} catch {
|
||||
rawBody = "";
|
||||
}
|
||||
return {
|
||||
success: res.ok,
|
||||
status: res.status,
|
||||
success: response.ok,
|
||||
status: response.status,
|
||||
latencyMs,
|
||||
responseBody: redactBody ? "<redacted: private target>" : rawBody,
|
||||
};
|
||||
|
||||
@@ -6,7 +6,8 @@
|
||||
|
||||
import crypto from "crypto";
|
||||
import { encrypt, decrypt } from "./db/encryption";
|
||||
import { parseAndValidateWebhookUrl } from "@/shared/network/outboundUrlGuardPolicy";
|
||||
import { OutboundUrlGuardError } from "@/shared/network/outboundUrlGuard";
|
||||
import { fetchWebhookUrl, type WebhookFetchOptions } from "@/shared/network/webhookFetch";
|
||||
import type { WebhookEvent } from "./webhooks/eventDescriptions";
|
||||
|
||||
export type { WebhookEvent };
|
||||
@@ -17,6 +18,10 @@ export interface WebhookPayload {
|
||||
data: Record<string, any>;
|
||||
}
|
||||
|
||||
/** DNS-resolve/fetch overrides — production callers never pass these; tests inject a fake
|
||||
* resolver and/or fetch to avoid real network access (#12569). */
|
||||
export type WebhookDeliveryOptions = Pick<WebhookFetchOptions, "lookup" | "fetchImpl">;
|
||||
|
||||
function signPayload(payload: string, secret: string): string {
|
||||
return `sha256=${crypto.createHmac("sha256", secret).update(payload).digest("hex")}`;
|
||||
}
|
||||
@@ -38,21 +43,24 @@ export function decryptMetadata(encrypted: string | null): Record<string, string
|
||||
|
||||
async function deliverRaw(
|
||||
url: string,
|
||||
body: Record<string, unknown>
|
||||
body: Record<string, unknown>,
|
||||
options?: WebhookDeliveryOptions
|
||||
): Promise<{ success: boolean; status: number; latencyMs: number; error?: string }> {
|
||||
const start = Date.now();
|
||||
try {
|
||||
parseAndValidateWebhookUrl(url);
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), 10_000);
|
||||
try {
|
||||
const res = await fetch(url, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json", "User-Agent": "OmniRoute-Webhook/1.0" },
|
||||
body: JSON.stringify(body),
|
||||
signal: controller.signal,
|
||||
});
|
||||
return { success: res.ok, status: res.status, latencyMs: Date.now() - start };
|
||||
const { response } = await fetchWebhookUrl(
|
||||
url,
|
||||
{
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json", "User-Agent": "OmniRoute-Webhook/1.0" },
|
||||
body: JSON.stringify(body),
|
||||
},
|
||||
{ ...options, signal: controller.signal }
|
||||
);
|
||||
return { success: response.ok, status: response.status, latencyMs: Date.now() - start };
|
||||
} finally {
|
||||
// Always clear the abort timer — on a non-timeout fetch error the previous code skipped
|
||||
// clearTimeout, leaving a dangling 10s timer (and AbortController) per failed call.
|
||||
@@ -72,13 +80,9 @@ export async function deliverWebhook(
|
||||
url: string,
|
||||
payload: WebhookPayload,
|
||||
secret?: string | null,
|
||||
maxRetries = 3
|
||||
maxRetries = 3,
|
||||
options?: WebhookDeliveryOptions
|
||||
): Promise<{ success: boolean; status: number; error?: string }> {
|
||||
try {
|
||||
parseAndValidateWebhookUrl(url);
|
||||
} catch (error: any) {
|
||||
return { success: false, status: 0, error: error.message || "Blocked outbound URL" };
|
||||
}
|
||||
const body = JSON.stringify(payload);
|
||||
const headers: Record<string, string> = {
|
||||
"Content-Type": "application/json",
|
||||
@@ -96,29 +100,31 @@ export async function deliverWebhook(
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), 10_000);
|
||||
|
||||
let res: Response;
|
||||
let response: Response;
|
||||
try {
|
||||
res = await fetch(url, {
|
||||
method: "POST",
|
||||
headers,
|
||||
body,
|
||||
signal: controller.signal,
|
||||
});
|
||||
({ response } = await fetchWebhookUrl(
|
||||
url,
|
||||
{ method: "POST", headers, body },
|
||||
{ ...options, signal: controller.signal }
|
||||
));
|
||||
} finally {
|
||||
// Clear the abort timer on every path — a non-timeout fetch error previously skipped
|
||||
// clearTimeout, leaking a dangling 10s timer + AbortController per failed attempt.
|
||||
clearTimeout(timeoutId);
|
||||
}
|
||||
|
||||
if (res.ok || res.status < 500) {
|
||||
return { success: res.ok, status: res.status };
|
||||
if (response.ok || response.status < 500) {
|
||||
return { success: response.ok, status: response.status };
|
||||
}
|
||||
|
||||
if (attempt < maxRetries) {
|
||||
await new Promise((r) => setTimeout(r, Math.pow(2, attempt) * 1000));
|
||||
}
|
||||
} catch (error: any) {
|
||||
if (attempt === maxRetries) {
|
||||
// A blocked outbound URL (private/metadata resolved address, or a redirect hop that
|
||||
// resolved to one) is never transient — fail closed immediately instead of burning
|
||||
// retries/backoff on something that will keep resolving the same way.
|
||||
if (attempt === maxRetries || error instanceof OutboundUrlGuardError) {
|
||||
return { success: false, status: 0, error: error.message || "Network error" };
|
||||
}
|
||||
await new Promise((r) => setTimeout(r, Math.pow(2, attempt) * 1000));
|
||||
|
||||
94
src/shared/network/dnsPinnedFetch.ts
Normal file
94
src/shared/network/dnsPinnedFetch.ts
Normal file
@@ -0,0 +1,94 @@
|
||||
import { isIP } from "node:net";
|
||||
import dns from "node:dns";
|
||||
import { Agent, fetch as undiciFetch } from "undici";
|
||||
|
||||
/**
|
||||
* Shared DNS-resolve-then-pin primitives (#12569). Originally written only for
|
||||
* `remoteImageFetch.ts` (GHSA-cmhj-wh2f-9cgx); extracted here so the webhook outbound-URL
|
||||
* guard (`webhookFetch.ts`) can reuse the exact same connection-pinning mechanism instead of
|
||||
* duplicating it. `remoteImageFetch.ts` re-exports `createPinnedFetch` from here for backward
|
||||
* compatibility with its existing import path.
|
||||
*/
|
||||
|
||||
export interface DnsLookupResult {
|
||||
address: string;
|
||||
family: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Minimal DNS lookup contract — matches the shape returned by
|
||||
* `node:dns/promises`.lookup(host, { all: true }). Exposed as an option so
|
||||
* tests can inject a fake resolver without touching real DNS.
|
||||
*/
|
||||
export type DnsLookup = (hostname: string) => Promise<DnsLookupResult[]>;
|
||||
|
||||
export const defaultDnsLookup: DnsLookup = (hostname) =>
|
||||
dns.promises.lookup(hostname, { all: true });
|
||||
|
||||
/** Strip literal IPv6 brackets: "[::1]" -> "::1". */
|
||||
export function bareHostname(hostname: string): string {
|
||||
return hostname.startsWith("[") && hostname.endsWith("]") ? hostname.slice(1, -1) : hostname;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve every DNS answer for a hostname, short-circuiting for an IP literal (which needs no
|
||||
* lookup — it already IS the connect-time address). Fails closed: a lookup error or an empty
|
||||
* answer set throws rather than being treated as "no restriction applies".
|
||||
*/
|
||||
export async function resolveHostnameAddresses(
|
||||
hostname: string,
|
||||
lookup: DnsLookup = defaultDnsLookup
|
||||
): Promise<DnsLookupResult[]> {
|
||||
const bare = bareHostname(hostname);
|
||||
if (!bare) return [];
|
||||
const literalFamily = isIP(bare);
|
||||
if (literalFamily) return [{ address: bare, family: literalFamily }];
|
||||
const resolved = await lookup(bare);
|
||||
if (!resolved.length) {
|
||||
throw new Error(`Host "${bare}" could not be resolved`);
|
||||
}
|
||||
return resolved;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a `fetch` bound to a single already-DNS-validated address, ignoring
|
||||
* whatever the hostname resolves to at connect time. Exported for direct
|
||||
* testing: this is the mechanism that closes the DNS-rebinding TOCTOU gap
|
||||
* (GHSA-cmhj-wh2f-9cgx) — a second, real DNS lookup at connect time could
|
||||
* otherwise return a different (possibly private) address than the one
|
||||
* validated up-front.
|
||||
*/
|
||||
export function createPinnedFetch(address: string, family: number): typeof fetch {
|
||||
const dispatcher = new Agent({
|
||||
connect: {
|
||||
// Node's `net.connect`/`tls.connect` invoke a custom `lookup` in one of
|
||||
// two incompatible shapes depending on `options.all`: modern Node
|
||||
// (autoSelectFamily / Happy Eyeballs, on by default since Node 18)
|
||||
// calls `lookup(hostname, { all: true, ... }, callback)` and requires
|
||||
// `callback(err, addresses[])` — an array of `{ address, family }`.
|
||||
// Only when `all` is falsy does it accept the single-address form
|
||||
// `callback(err, address, family)`. Handling only the single-address
|
||||
// form here (as an earlier draft did) throws `ERR_INVALID_IP_ADDRESS`
|
||||
// for every real request once autoSelectFamily kicks in, silently
|
||||
// breaking every pinned fetch — verified by
|
||||
// `tests/unit/remote-image-fetch-pin-dns-connection.test.ts`.
|
||||
lookup: (_hostname, options, callback) => {
|
||||
if (options && typeof options === "object" && "all" in options && options.all) {
|
||||
callback(null, [{ address, family }]);
|
||||
return;
|
||||
}
|
||||
callback(null, address, family);
|
||||
},
|
||||
},
|
||||
});
|
||||
return (async (input, init) => {
|
||||
try {
|
||||
return (await undiciFetch(input as string | URL, {
|
||||
...(init as Parameters<typeof undiciFetch>[1]),
|
||||
dispatcher,
|
||||
})) as unknown as Response;
|
||||
} finally {
|
||||
await dispatcher.close();
|
||||
}
|
||||
}) as typeof fetch;
|
||||
}
|
||||
@@ -1,6 +1,5 @@
|
||||
import { isIP } from "node:net";
|
||||
import dns from "node:dns";
|
||||
import { Agent, fetch as undiciFetch } from "undici";
|
||||
import {
|
||||
type OutboundUrlGuardMode,
|
||||
isPrivateHost,
|
||||
@@ -9,6 +8,13 @@ import {
|
||||
parseOutboundUrl,
|
||||
} from "@/shared/network/outboundUrlGuard";
|
||||
import { getProviderOutboundGuard } from "@/shared/network/outboundUrlGuardPolicy";
|
||||
// #12569: `createPinnedFetch` now lives in the shared `dnsPinnedFetch.ts` module so the
|
||||
// webhook outbound-URL guard can reuse the exact same connection-pinning mechanism instead of
|
||||
// duplicating it. Re-exported here for backward compatibility with existing importers of
|
||||
// `@/shared/network/remoteImageFetch`.
|
||||
import { createPinnedFetch } from "@/shared/network/dnsPinnedFetch";
|
||||
|
||||
export { createPinnedFetch };
|
||||
|
||||
const DEFAULT_MAX_REMOTE_IMAGE_BYTES = 20 * 1024 * 1024;
|
||||
const DEFAULT_MAX_REDIRECTS = 3;
|
||||
@@ -95,48 +101,6 @@ async function assertHostnameResolvesPublic(
|
||||
}
|
||||
return resolved;
|
||||
}
|
||||
/**
|
||||
* Build a `fetch` bound to a single already-DNS-validated address, ignoring
|
||||
* whatever the hostname resolves to at connect time. Exported for direct
|
||||
* testing: this is the mechanism that closes the DNS-rebinding TOCTOU gap
|
||||
* (GHSA-cmhj-wh2f-9cgx) — a second, real DNS lookup at connect time could
|
||||
* otherwise return a different (possibly private) address than the one
|
||||
* `assertHostnameResolvesPublic` validated.
|
||||
*/
|
||||
export function createPinnedFetch(address: string, family: number): typeof fetch {
|
||||
const dispatcher = new Agent({
|
||||
connect: {
|
||||
// Node's `net.connect`/`tls.connect` invoke a custom `lookup` in one of
|
||||
// two incompatible shapes depending on `options.all`: modern Node
|
||||
// (autoSelectFamily / Happy Eyeballs, on by default since Node 18)
|
||||
// calls `lookup(hostname, { all: true, ... }, callback)` and requires
|
||||
// `callback(err, addresses[])` — an array of `{ address, family }`.
|
||||
// Only when `all` is falsy does it accept the single-address form
|
||||
// `callback(err, address, family)`. Handling only the single-address
|
||||
// form here (as an earlier draft did) throws `ERR_INVALID_IP_ADDRESS`
|
||||
// for every real request once autoSelectFamily kicks in, silently
|
||||
// breaking every pinned fetch — verified by
|
||||
// `tests/unit/remote-image-fetch-pin-dns-connection.test.ts`.
|
||||
lookup: (_hostname, options, callback) => {
|
||||
if (options && typeof options === "object" && "all" in options && options.all) {
|
||||
callback(null, [{ address, family }]);
|
||||
return;
|
||||
}
|
||||
callback(null, address, family);
|
||||
},
|
||||
},
|
||||
});
|
||||
return (async (input, init) => {
|
||||
try {
|
||||
return (await undiciFetch(input as string | URL, {
|
||||
...(init as Parameters<typeof undiciFetch>[1]),
|
||||
dispatcher,
|
||||
})) as unknown as Response;
|
||||
} finally {
|
||||
await dispatcher.close();
|
||||
}
|
||||
}) as typeof fetch;
|
||||
}
|
||||
function combineSignals(signal: AbortSignal | undefined, timeoutMs: number) {
|
||||
const timeoutSignal = AbortSignal.timeout(timeoutMs);
|
||||
if (!signal) return timeoutSignal;
|
||||
|
||||
167
src/shared/network/webhookFetch.ts
Normal file
167
src/shared/network/webhookFetch.ts
Normal file
@@ -0,0 +1,167 @@
|
||||
import {
|
||||
createPinnedFetch,
|
||||
defaultDnsLookup,
|
||||
resolveHostnameAddresses,
|
||||
type DnsLookup,
|
||||
type DnsLookupResult,
|
||||
} from "@/shared/network/dnsPinnedFetch";
|
||||
import {
|
||||
isCloudMetadataHost,
|
||||
isPrivateHost,
|
||||
OutboundUrlGuardError,
|
||||
parseOutboundUrl,
|
||||
PROVIDER_URL_BLOCKED_MESSAGE,
|
||||
} from "@/shared/network/outboundUrlGuard";
|
||||
import { arePrivateProviderUrlsAllowed } from "@/shared/network/outboundUrlGuardPolicy";
|
||||
|
||||
/**
|
||||
* #12569 — DNS-resolve-then-pin fetch for webhook outbound calls (custom webhook delivery +
|
||||
* the webhook test-diagnostics endpoint). `parseAndValidateWebhookUrl` in
|
||||
* `outboundUrlGuardPolicy.ts` only classifies the literal hostname STRING, so a hostname an
|
||||
* attacker controls (DNS pointed at 169.254.169.254 / an RFC1918 address) passed that guard
|
||||
* and reached the real `fetch()` unmodified. This module resolves DNS up front, rejects any
|
||||
* resolved answer that is cloud-metadata (always) or private (unless the private-provider-URL
|
||||
* opt-in is on), pins the connection to a validated address, and revalidates every redirect
|
||||
* hop the same way — a public host answering 302 to an internal address no longer escapes
|
||||
* the guard.
|
||||
*/
|
||||
|
||||
const DEFAULT_MAX_REDIRECTS = 3;
|
||||
|
||||
export interface WebhookFetchOptions {
|
||||
/** DNS resolver override. Tests inject a fake resolver to avoid real network lookups. */
|
||||
lookup?: DnsLookup;
|
||||
/** Fetch override. Takes priority over connection pinning — the mockable escape hatch used
|
||||
* by existing tests that stub `globalThis.fetch`. */
|
||||
fetchImpl?: typeof fetch;
|
||||
/** Pin the connection to the validated DNS answer. Default true — this is the mechanism
|
||||
* that closes the DNS-rebinding TOCTOU gap. */
|
||||
pinDns?: boolean;
|
||||
maxRedirects?: number;
|
||||
signal?: AbortSignal;
|
||||
}
|
||||
|
||||
export interface WebhookFetchResult {
|
||||
response: Response;
|
||||
finalUrl: string;
|
||||
/** True when a resolved hop is a private address explicitly allowed via opt-in — the
|
||||
* caller must not surface the upstream response body for such a target (#3269). */
|
||||
redactBody: boolean;
|
||||
}
|
||||
|
||||
/** Reject a resolved address set that includes a metadata or (non-opted-in) private IP. */
|
||||
function assertAddressesAllowed(addresses: DnsLookupResult[], url: URL): boolean {
|
||||
const allowPrivate = arePrivateProviderUrlsAllowed();
|
||||
let sawPrivate = false;
|
||||
for (const { address } of addresses) {
|
||||
if (isCloudMetadataHost(address)) {
|
||||
throw new OutboundUrlGuardError(PROVIDER_URL_BLOCKED_MESSAGE, {
|
||||
code: "OUTBOUND_URL_GUARD_BLOCKED",
|
||||
url: url.toString(),
|
||||
hostname: address,
|
||||
});
|
||||
}
|
||||
if (isPrivateHost(address)) {
|
||||
if (!allowPrivate) {
|
||||
throw new OutboundUrlGuardError(PROVIDER_URL_BLOCKED_MESSAGE, {
|
||||
code: "OUTBOUND_URL_GUARD_BLOCKED",
|
||||
url: url.toString(),
|
||||
hostname: address,
|
||||
});
|
||||
}
|
||||
sawPrivate = true;
|
||||
}
|
||||
}
|
||||
return sawPrivate;
|
||||
}
|
||||
|
||||
async function resolveHop(
|
||||
currentUrl: string | URL,
|
||||
lookup: DnsLookup
|
||||
): Promise<{ url: URL; addresses: DnsLookupResult[]; redactBody: boolean }> {
|
||||
const url = parseOutboundUrl(currentUrl);
|
||||
let addresses: DnsLookupResult[];
|
||||
try {
|
||||
addresses = await resolveHostnameAddresses(url.hostname, lookup);
|
||||
} catch {
|
||||
throw new OutboundUrlGuardError("Webhook host could not be resolved (blocked)", {
|
||||
code: "OUTBOUND_URL_GUARD_BLOCKED",
|
||||
url: url.toString(),
|
||||
hostname: url.hostname || null,
|
||||
});
|
||||
}
|
||||
const redactBody = assertAddressesAllowed(addresses, url);
|
||||
return { url, addresses, redactBody };
|
||||
}
|
||||
|
||||
function pickFetchImpl(
|
||||
fetchImpl: typeof fetch | undefined,
|
||||
pinDns: boolean,
|
||||
addresses: DnsLookupResult[]
|
||||
): typeof fetch {
|
||||
if (fetchImpl) return fetchImpl;
|
||||
if (pinDns && addresses.length) return createPinnedFetch(addresses[0].address, addresses[0].family);
|
||||
return fetch;
|
||||
}
|
||||
|
||||
function nextRedirectUrl(
|
||||
response: Response,
|
||||
currentUrl: URL,
|
||||
redirectCount: number,
|
||||
maxRedirects: number
|
||||
): URL {
|
||||
const location = response.headers.get("location");
|
||||
if (!location) {
|
||||
throw new OutboundUrlGuardError("Webhook redirect missing Location header", {
|
||||
code: "OUTBOUND_URL_INVALID",
|
||||
url: currentUrl.toString(),
|
||||
});
|
||||
}
|
||||
if (redirectCount >= maxRedirects) {
|
||||
throw new OutboundUrlGuardError(`Webhook exceeded ${maxRedirects} redirect limit`, {
|
||||
code: "OUTBOUND_URL_GUARD_BLOCKED",
|
||||
url: currentUrl.toString(),
|
||||
});
|
||||
}
|
||||
return new URL(location, currentUrl);
|
||||
}
|
||||
|
||||
/**
|
||||
* DNS-resolve-then-pin POST/GET for a webhook URL, following redirects manually and
|
||||
* revalidating DNS at every hop. Throws `OutboundUrlGuardError` when the target (or a
|
||||
* redirect target) resolves to a blocked address.
|
||||
*/
|
||||
export async function fetchWebhookUrl(
|
||||
input: string,
|
||||
init: RequestInit,
|
||||
options: WebhookFetchOptions = {}
|
||||
): Promise<WebhookFetchResult> {
|
||||
const lookup = options.lookup ?? defaultDnsLookup;
|
||||
const maxRedirects = options.maxRedirects ?? DEFAULT_MAX_REDIRECTS;
|
||||
const pinDns = options.pinDns !== false;
|
||||
let currentUrl: string | URL = input;
|
||||
let redactBody = false;
|
||||
|
||||
for (let redirectCount = 0; redirectCount <= maxRedirects; redirectCount++) {
|
||||
const hop = await resolveHop(currentUrl, lookup);
|
||||
redactBody = redactBody || hop.redactBody;
|
||||
const fetchImpl = pickFetchImpl(options.fetchImpl, pinDns, hop.addresses);
|
||||
const response = await fetchImpl(hop.url.toString(), {
|
||||
...init,
|
||||
redirect: "manual",
|
||||
signal: options.signal,
|
||||
});
|
||||
|
||||
if (response.status >= 300 && response.status < 400) {
|
||||
currentUrl = nextRedirectUrl(response, hop.url, redirectCount, maxRedirects);
|
||||
continue;
|
||||
}
|
||||
|
||||
return { response, finalUrl: hop.url.toString(), redactBody };
|
||||
}
|
||||
|
||||
throw new OutboundUrlGuardError(`Webhook exceeded ${maxRedirects} redirect limit`, {
|
||||
code: "OUTBOUND_URL_GUARD_BLOCKED",
|
||||
url: String(input),
|
||||
});
|
||||
}
|
||||
@@ -8,10 +8,15 @@ const { deliverWebhook } = await import("../../src/lib/webhookDispatcher.ts");
|
||||
// called clearTimeout on the success path, so a non-timeout fetch rejection
|
||||
// (ECONNREFUSED, DNS failure, etc.) skipped clearTimeout, leaking a live 10s timer
|
||||
// + AbortController per failed delivery. The fix clears the timer in a `finally`.
|
||||
//
|
||||
// #12569: deliverWebhook now DNS-resolves and pins the connection before dispatch, so a
|
||||
// `globalThis.fetch` stub alone no longer intercepts the outbound call (the pinned fetch talks
|
||||
// to undici directly). Inject a fake `lookup` (no real DNS) and `fetchImpl` (the documented
|
||||
// escape hatch — see `WebhookDeliveryOptions`) instead, so this test stays deterministic and
|
||||
// network-free while still exercising the exact "fetch rejects" path it targets.
|
||||
test("deliverWebhook clears the abort timer even when fetch rejects", async () => {
|
||||
const realSetTimeout = globalThis.setTimeout;
|
||||
const realClearTimeout = globalThis.clearTimeout;
|
||||
const realFetch = globalThis.fetch;
|
||||
|
||||
const abortTimerIds = new Set<unknown>();
|
||||
const clearedIds = new Set<unknown>();
|
||||
@@ -26,17 +31,20 @@ test("deliverWebhook clears the abort timer even when fetch rejects", async () =
|
||||
clearedIds.add(id);
|
||||
return realClearTimeout(id);
|
||||
}) as typeof clearTimeout;
|
||||
// Non-timeout network failure — the exact path that previously skipped clearTimeout.
|
||||
globalThis.fetch = (async () => {
|
||||
throw new Error("ECONNREFUSED");
|
||||
}) as typeof fetch;
|
||||
|
||||
try {
|
||||
const res = await deliverWebhook(
|
||||
"https://example.com/webhook",
|
||||
{ event: "test.event" as any, timestamp: new Date().toISOString(), data: {} },
|
||||
null,
|
||||
0 // maxRetries=0 → single attempt, no exponential-backoff timers
|
||||
0, // maxRetries=0 → single attempt, no exponential-backoff timers
|
||||
{
|
||||
lookup: async () => [{ address: "203.0.113.5", family: 4 }],
|
||||
// Non-timeout network failure — the exact path that previously skipped clearTimeout.
|
||||
fetchImpl: async () => {
|
||||
throw new Error("ECONNREFUSED");
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
assert.equal(res.success, false, "delivery should fail when fetch rejects");
|
||||
@@ -52,6 +60,5 @@ test("deliverWebhook clears the abort timer even when fetch rejects", async () =
|
||||
} finally {
|
||||
globalThis.setTimeout = realSetTimeout;
|
||||
globalThis.clearTimeout = realClearTimeout;
|
||||
globalThis.fetch = realFetch;
|
||||
}
|
||||
});
|
||||
|
||||
135
tests/unit/webhook-dns-rebinding-ssrf-12569.test.ts
Normal file
135
tests/unit/webhook-dns-rebinding-ssrf-12569.test.ts
Normal file
@@ -0,0 +1,135 @@
|
||||
/**
|
||||
* Regression for issue #12569: the webhook outbound-URL guard
|
||||
* (`parseAndValidateWebhookUrl`, `isPrivateHost`, `isCloudMetadataHost`) classified only the
|
||||
* literal hostname STRING in the configured webhook URL. It never resolved DNS before
|
||||
* deciding a target was public, so a domain an attacker controls (DNS A record pointed at
|
||||
* 169.254.169.254 / an RFC1918 address) passed the guard, and the real `fetch()` that
|
||||
* followed resolved DNS itself and reached the internal target (DNS rebinding).
|
||||
*
|
||||
* Fixed by `fetchWebhookUrl` (`src/shared/network/webhookFetch.ts`), which resolves DNS
|
||||
* up-front, rejects any resolved answer that is cloud-metadata/private, and pins the
|
||||
* connection to the validated address (so a *second*, real DNS lookup at connect time cannot
|
||||
* rebind to a different address either).
|
||||
*
|
||||
* Run with:
|
||||
* node --import tsx/esm --test tests/unit/webhook-dns-rebinding-ssrf-12569.test.ts
|
||||
*/
|
||||
|
||||
import { describe, it, mock, after } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import dns from "node:dns";
|
||||
|
||||
import { deliverWebhook } from "../../src/lib/webhookDispatcher.ts";
|
||||
|
||||
const REBOUND_HOSTNAME = "evil.example.com";
|
||||
const IMDS_ADDRESS = "169.254.169.254";
|
||||
|
||||
const originalLookup = dns.promises.lookup;
|
||||
mock.method(
|
||||
dns.promises,
|
||||
"lookup",
|
||||
async (hostname: string): Promise<dns.LookupAddress[]> => {
|
||||
if (hostname === REBOUND_HOSTNAME) {
|
||||
return [{ address: IMDS_ADDRESS, family: 4 }];
|
||||
}
|
||||
return originalLookup(hostname, { all: true });
|
||||
}
|
||||
);
|
||||
|
||||
after(() => {
|
||||
mock.restoreAll();
|
||||
});
|
||||
|
||||
describe("#12569 — webhook outbound guard is hostname-string-only (DNS rebinding)", () => {
|
||||
it("does NOT let a hostname that resolves to the cloud-metadata IP reach fetch()", async () => {
|
||||
const fetchCalls: string[] = [];
|
||||
const originalFetch = globalThis.fetch;
|
||||
// @ts-expect-error - stubbing global fetch for the probe
|
||||
globalThis.fetch = async (input: string) => {
|
||||
fetchCalls.push(String(input));
|
||||
return new Response("ok", { status: 200 });
|
||||
};
|
||||
|
||||
try {
|
||||
const res = await deliverWebhook(
|
||||
`http://${REBOUND_HOSTNAME}/hook`,
|
||||
{ event: "test.ping", timestamp: new Date().toISOString(), data: {} },
|
||||
"secret"
|
||||
);
|
||||
|
||||
assert.equal(
|
||||
fetchCalls.length,
|
||||
0,
|
||||
`guard should have blocked dispatch to a hostname resolving to ${IMDS_ADDRESS}, ` +
|
||||
`but fetch() was called with: ${JSON.stringify(fetchCalls)}`
|
||||
);
|
||||
assert.equal(res.success, false);
|
||||
} finally {
|
||||
globalThis.fetch = originalFetch;
|
||||
}
|
||||
});
|
||||
|
||||
it("blocks a hostname that resolves to an RFC1918 address, without retrying", async () => {
|
||||
const start = Date.now();
|
||||
const res = await deliverWebhook(
|
||||
"http://rebind-to-lan.example.com/hook",
|
||||
{ event: "test.ping", timestamp: new Date().toISOString(), data: {} },
|
||||
null,
|
||||
3,
|
||||
{ lookup: async () => [{ address: "10.1.2.3", family: 4 }] }
|
||||
);
|
||||
const elapsedMs = Date.now() - start;
|
||||
|
||||
assert.equal(res.success, false);
|
||||
assert.ok(
|
||||
typeof res.error === "string" && /private|blocked|local/i.test(res.error),
|
||||
`expected guard error, got: ${res.error}`
|
||||
);
|
||||
// A guard-blocked verdict must fail fast — no exponential-backoff retries (1s+2s+4s) for
|
||||
// something that will keep resolving the same way.
|
||||
assert.ok(elapsedMs < 900, `blocked delivery must not retry with backoff (took ${elapsedMs}ms)`);
|
||||
});
|
||||
|
||||
it("blocks when any of several resolved addresses is private (multi-A trick)", async () => {
|
||||
const fetchCalls: string[] = [];
|
||||
const res = await deliverWebhook(
|
||||
"http://multi-answer.example.com/hook",
|
||||
{ event: "test.ping", timestamp: new Date().toISOString(), data: {} },
|
||||
null,
|
||||
0,
|
||||
{
|
||||
lookup: async () => [
|
||||
{ address: "203.0.113.5", family: 4 },
|
||||
{ address: "169.254.169.254", family: 4 },
|
||||
],
|
||||
fetchImpl: async (input: string | URL) => {
|
||||
fetchCalls.push(String(input));
|
||||
return new Response("ok", { status: 200 });
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
assert.equal(res.success, false);
|
||||
assert.equal(fetchCalls.length, 0, "fetch must never fire when any resolved IP is blocked");
|
||||
});
|
||||
|
||||
it("allows a hostname that resolves only to public addresses", async () => {
|
||||
const fetchCalls: string[] = [];
|
||||
const res = await deliverWebhook(
|
||||
"http://public-looking.example.com/hook",
|
||||
{ event: "test.ping", timestamp: new Date().toISOString(), data: {} },
|
||||
null,
|
||||
0,
|
||||
{
|
||||
lookup: async () => [{ address: "203.0.113.5", family: 4 }],
|
||||
fetchImpl: async (input: string | URL) => {
|
||||
fetchCalls.push(String(input));
|
||||
return new Response("ok", { status: 200 });
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
assert.equal(res.success, true);
|
||||
assert.equal(fetchCalls.length, 1);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user