From efb7a7fdc1af8d8238095c6469f1cbcf126c50a4 Mon Sep 17 00:00:00 2001 From: Gilad Resisi Date: Fri, 3 Jul 2026 12:01:03 +0700 Subject: [PATCH] fix(security): block literal private/loopback IPs in SSRF-safe dispatcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ssrfSafeDispatcher enforced its allow-list only inside the undici `connect.lookup` hook. But Node's net.connect only calls `lookup` for hostnames that need DNS resolution — for a literal-IP host it connects directly and never invokes lookup. So any URL with a literal private, loopback, or link-local IP (http://192.168.1.1/, http://127.0.0.1/, http://169.254.169.254/, http://[::1]/, ...) bypassed the guard entirely. This is an SSRF hole affecting every consumer of the dispatcher: the uploadFromUrl agent tool, the public /upload-from-url API, and webhook delivery. Any authenticated API user could make the server issue requests to hosts on its private network (cloud metadata endpoints, internal services, localhost-bound databases) and read back non-2xx / error signal. Fix: move the literal-IP check into a custom `connect` (undici buildConnector), which runs for every connection regardless of DNS. The hostname DNS-pinning path is kept as `pinnedConnector.lookup` so hostnames resolving to private IPs stay blocked too (the TOCTOU protection from GHSA-f7jj-p389-4w45 is preserved). Reproduced (pre-fix), live via the uploadFromUrl agent tool over MCP: http://192.168.1.1/a.jpg -> {"error":"Failed to fetch URL"} i.e. fetch SUCCEEDED — the server connected to the private host and got a response back (the "!response.ok" branch), proving the round-trip happened. Validated (post-fix), same live call: http://192.168.1.1/a.jpg -> {"error":"Failed to upload media from URL: fetch failed"} i.e. the connection is now rejected at the dispatcher before any socket opens (the catch branch). With the companion err.cause change applied it reads "... fetch failed (Blocked IP)". No regression (same live tool): https://picsum.photos/200/300.jpg -> {"id","path"} (public image still uploads) https://httpbin.org/status/404 -> "Failed to fetch URL" (ordinary non-2xx path intact) Also verified with a standalone undici repro: literal 192.168.1.1, 127.0.0.1, 169.254.169.254, [::1] and hostname localhost are all rejected with "Blocked IP", while example.com still connects. Co-Authored-By: Claude Fable 5 --- .../src/dtos/webhooks/ssrf.safe.dispatcher.ts | 58 ++++++++++--------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/libraries/nestjs-libraries/src/dtos/webhooks/ssrf.safe.dispatcher.ts b/libraries/nestjs-libraries/src/dtos/webhooks/ssrf.safe.dispatcher.ts index dfd69abee8..0a1ec035c1 100644 --- a/libraries/nestjs-libraries/src/dtos/webhooks/ssrf.safe.dispatcher.ts +++ b/libraries/nestjs-libraries/src/dtos/webhooks/ssrf.safe.dispatcher.ts @@ -1,4 +1,4 @@ -import { Agent } from 'undici'; +import { Agent, buildConnector } from 'undici'; import dns from 'node:dns'; import net from 'node:net'; import { isBlockedIp } from './webhook.url.validator'; @@ -6,35 +6,39 @@ import { isBlockedIp } from './webhook.url.validator'; // Pins DNS resolution: every resolved IP is checked with `isBlockedIp` and // the caller (undici) connects to that same set. Closes the TOCTOU window // `isSafePublicHttpsUrl` alone leaves open (see GHSA-f7jj-p389-4w45). -export const ssrfSafeDispatcher = new Agent({ - connect: { - lookup(hostname, options, callback) { - if (net.isIP(hostname)) { - const family = net.isIP(hostname); - if (isBlockedIp(hostname)) { - return callback(new Error('Blocked IP'), '', 0); +// +// The `lookup` hook only runs for hostnames that need DNS resolution; Node's +// net.connect skips it for literal-IP hosts (e.g. http://192.168.1.1/), so a +// literal private/loopback IP would otherwise bypass the guard entirely. We +// therefore also check literal IPs in a custom `connect`, which runs for every +// connection. +const pinnedConnector = buildConnector({ + lookup(hostname, options, callback) { + dns.lookup(hostname, options, (err, address: any, family: any) => { + if (err) return callback(err, '', 0); + if (Array.isArray(address)) { + for (const entry of address) { + if (isBlockedIp(entry.address)) { + return callback(new Error('Blocked IP'), '', 0); + } } - return options && (options as any).all - ? callback(null, [{ address: hostname, family }] as any, family) - : callback(null, hostname, family); + return callback(null, address as any, 0); + } + if (isBlockedIp(address)) { + return callback(new Error('Blocked IP'), '', 0); } + callback(null, address, family); + }); + }, +}); - dns.lookup(hostname, options, (err, address: any, family: any) => { - if (err) return callback(err, '', 0); - if (Array.isArray(address)) { - for (const entry of address) { - if (isBlockedIp(entry.address)) { - return callback(new Error('Blocked IP'), '', 0); - } - } - return callback(null, address as any, 0); - } - if (isBlockedIp(address)) { - return callback(new Error('Blocked IP'), '', 0); - } - callback(null, address, family); - }); - }, +export const ssrfSafeDispatcher = new Agent({ + connect(options, callback) { + const hostname = (options.hostname || '').replace(/^\[|\]$/g, ''); + if (net.isIP(hostname) && isBlockedIp(hostname)) { + return callback(new Error('Blocked IP'), null); + } + return pinnedConnector(options, callback); }, });