diff --git a/.changeset/webhook-ssrf-dns-check.md b/.changeset/webhook-ssrf-dns-check.md new file mode 100644 index 0000000000..db0fd457ad --- /dev/null +++ b/.changeset/webhook-ssrf-dns-check.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/api': patch +--- + +fix: Block generic and incident.io webhooks whose hostname resolves to a private or reserved address. The check runs on the address actually dialed, so it also covers DNS rebinding. Self-hosted setups that send webhooks to internal hostnames must add them to `WEBHOOK_HOSTNAME_ALLOWLIST`. diff --git a/packages/api/jest.setup.ts b/packages/api/jest.setup.ts index 3e270f01e9..915b267e7a 100644 --- a/packages/api/jest.setup.ts +++ b/packages/api/jest.setup.ts @@ -40,3 +40,10 @@ global.fetch = jest.fn().mockResolvedValue({ text: jest.fn().mockResolvedValue(''), json: jest.fn().mockResolvedValue({}), } as any); + +// Webhook delivery uses undici's fetch; route it to the global mock so tests +// stub and assert on `global.fetch` as before. +jest.mock('undici', () => ({ + ...jest.requireActual('undici'), + fetch: (...args: Parameters) => global.fetch(...args), +})); diff --git a/packages/api/package.json b/packages/api/package.json index 390af6805e..953b456c19 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -61,6 +61,7 @@ "protobufjs": "^7.6.5", "serialize-error": "^8.1.0", "sqlstring": "^2.3.3", + "undici": "^7.29.0", "uuid": "^8.3.2", "zod": "3.25", "zod-express-middleware": "^1.4.0" diff --git a/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts b/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts index 67bf9b0115..8a7045229d 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts @@ -5037,6 +5037,7 @@ describe('checkAlerts', () => { 'Idempotency-Key': expect.any(String), }), signal: expect.any(AbortSignal), + dispatcher: expect.anything(), }); }); diff --git a/packages/api/src/tasks/checkAlerts/transports/__tests__/generic.test.ts b/packages/api/src/tasks/checkAlerts/transports/__tests__/generic.test.ts index d23912e4d4..a634d479c8 100644 --- a/packages/api/src/tasks/checkAlerts/transports/__tests__/generic.test.ts +++ b/packages/api/src/tasks/checkAlerts/transports/__tests__/generic.test.ts @@ -2,6 +2,7 @@ import { DEFAULT_INCIDENT_IO_WEBHOOK_BODY, WebhookService, } from '@hyperdx/common-utils/dist/types'; +import dns from 'dns'; import { ObjectId } from 'mongodb'; import { AlertState } from '@/models/alert'; @@ -12,6 +13,7 @@ import { handleSendGenericWebhook, } from '@/tasks/checkAlerts/transports/generic'; import type { Message } from '@/tasks/checkAlerts/transports/types'; +import { WebhookUrlValidationError } from '@/utils/validators'; const message: Message = { hdxLink: 'https://example.test/alert', @@ -91,6 +93,48 @@ describe('handleSendGenericWebhook — per-attempt timeout', () => { }, 2000); // short test-level timeout: a regression here should fail fast, not stall on Jest's default 30s }); +describe('handleSendGenericWebhook — DNS resolution', () => { + afterEach(() => jest.restoreAllMocks()); + + it('blocks a public hostname that resolves to loopback, without retrying', async () => { + // The real undici fetch + Agent pair that production uses. + jest + .mocked(global.fetch) + .mockImplementationOnce(jest.requireActual('undici').fetch); + const lookupSpy = jest + .spyOn(dns, 'lookup') + .mockImplementation((...args: any[]) => + args[2](null, [{ address: '127.0.0.1', family: 4 }]), + ); + const loopbackChannel: any = { + type: 'webhook', + channel: { ...webhook, url: 'http://127-0-0-1.nip.io/' }, + }; + + await expect( + handleSendGenericWebhook(loopbackChannel, message), + ).rejects.toThrow(WebhookUrlValidationError); + expect(lookupSpy).toHaveBeenCalledTimes(1); + }, 2000); + + it('unwraps a block nested deeper in the cause chain, without retrying', async () => { + const blocked = new WebhookUrlValidationError('blocked'); + jest + .mocked(global.fetch) + .mockClear() + .mockRejectedValueOnce( + new TypeError('fetch failed', { + cause: new Error('connect failed', { cause: blocked }), + }), + ); + + await expect(handleSendGenericWebhook(channel, message)).rejects.toBe( + blocked, + ); + expect(global.fetch).toHaveBeenCalledTimes(1); + }, 2000); +}); + describe('buildWebhookTemplateVariables', () => { it('exposes the enriched variables alongside the original set', () => { const vars = buildWebhookTemplateVariables({ diff --git a/packages/api/src/tasks/checkAlerts/transports/generic.ts b/packages/api/src/tasks/checkAlerts/transports/generic.ts index 035ff547f0..b4b2f04769 100644 --- a/packages/api/src/tasks/checkAlerts/transports/generic.ts +++ b/packages/api/src/tasks/checkAlerts/transports/generic.ts @@ -8,6 +8,7 @@ import Handlebars from 'handlebars'; import _ from 'lodash'; import { performance } from 'perf_hooks'; import { serializeError } from 'serialize-error'; +import { Agent, fetch } from 'undici'; import { IWebhook } from '@/models/webhook'; import { @@ -24,9 +25,12 @@ import logger from '@/utils/logger'; import { withRetry } from '@/utils/retry'; import { validateWebhookUrl, + webhookLookup, WebhookUrlValidationError, } from '@/utils/validators'; +const webhookDispatcher = new Agent({ connect: { lookup: webhookLookup } }); + // Webhook delivery is the last (and most failure-prone) hop of an alert. It // happens in the background task, so failures only show up in logs today. // `service` and `outcome` are bounded enums (see agent_docs/observability.md). @@ -261,6 +265,13 @@ const sendGenericWebhook = async ( headers, body, signal: attemptSignal, + dispatcher: webhookDispatcher, + }).catch(e => { + // Surface a connect-time block as the validation error (a 400), not "fetch failed". + for (let c = e; c; c = c.cause) { + if (c instanceof WebhookUrlValidationError) throw c; + } + throw e; }); // Disallow redirects to avoid redirect-based SSRF. diff --git a/packages/api/src/utils/__tests__/validators.test.ts b/packages/api/src/utils/__tests__/validators.test.ts index fd9347006d..8800d7ff48 100644 --- a/packages/api/src/utils/__tests__/validators.test.ts +++ b/packages/api/src/utils/__tests__/validators.test.ts @@ -1,4 +1,5 @@ import { WebhookService } from '@hyperdx/common-utils/dist/types'; +import dns from 'dns'; import { createHandlebarsWithHelpers } from '@/tasks/checkAlerts/transports'; import * as validators from '@/utils/validators'; @@ -98,6 +99,10 @@ describe('validators', () => { ['mapped public address', '::ffff:8.8.8.8', false], ['mapped private address in hexadecimal form', '::ffff:ac10:1', true], ['mapped public address in hexadecimal form', '::ffff:808:808', false], + ['NAT64 loopback address', '64:ff9b::7f00:1', true], + ['NAT64 private address in dotted form', '64:ff9b::10.0.0.1', true], + ['NAT64 public address', '64:ff9b::808:808', false], + ['local-use NAT64 address', '64:ff9b:1::808:808', true], ['private IPv4 address with a CIDR suffix', '127.0.0.1/8', true], ['scoped IPv6 link-local address', 'fe80::1%lo0', true], ])('%s (%s)', (_description, ip, expected) => { @@ -128,28 +133,28 @@ describe('validators', () => { }); }); - describe('validateWebhookUrl', () => { - const loadValidatorsWithConfig = (config: { - clickhouseHost?: string; - hostnameAllowlist?: string; - }): typeof validators => { - let isolatedValidators: typeof validators | undefined; - - jest.isolateModules(() => { - jest.doMock('@/config', () => ({ - CLICKHOUSE_HOST: config.clickhouseHost, - MONGO_URI: undefined, - WEBHOOK_HOSTNAME_ALLOWLIST: config.hostnameAllowlist ?? '', - })); - - // eslint-disable-next-line @typescript-eslint/no-require-imports, n/no-missing-require - isolatedValidators = require('@/utils/validators'); - }); - jest.dontMock('@/config'); + const loadValidatorsWithConfig = (config: { + clickhouseHost?: string; + hostnameAllowlist?: string; + }): typeof validators => { + let isolatedValidators: typeof validators | undefined; + + jest.isolateModules(() => { + jest.doMock('@/config', () => ({ + CLICKHOUSE_HOST: config.clickhouseHost, + MONGO_URI: undefined, + WEBHOOK_HOSTNAME_ALLOWLIST: config.hostnameAllowlist ?? '', + })); + + // eslint-disable-next-line @typescript-eslint/no-require-imports, n/no-missing-require + isolatedValidators = require('@/utils/validators'); + }); + jest.dontMock('@/config'); - return isolatedValidators!; - }; + return isolatedValidators!; + }; + describe('validateWebhookUrl', () => { it.each([ [ 'generic webhook', @@ -322,6 +327,81 @@ describe('validators', () => { }); }); + describe('webhookLookup', () => { + const lookup = ( + v: typeof validators, + hostname: string, + all = false, + ): Promise => + new Promise((resolve, reject) => + v.webhookLookup(hostname, { all }, (err, address) => + err ? reject(err) : resolve(address), + ), + ); + + const mockLookup = (err: Error | null, addresses: dns.LookupAddress[]) => + jest + .spyOn(dns, 'lookup') + .mockImplementation((...args: any[]) => args[2](err, addresses)); + + afterEach(() => jest.restoreAllMocks()); + + it('blocks a hostname that resolves to a private address', async () => { + await expect(lookup(validators, 'localhost')).rejects.toThrow( + validators.WebhookUrlValidationError, + ); + }); + + it('returns public addresses in the shape the caller asked for', async () => { + mockLookup(null, [{ address: '93.184.216.34', family: 4 }]); + + await expect(lookup(validators, 'example.com')).resolves.toBe( + '93.184.216.34', + ); + await expect(lookup(validators, 'example.com', true)).resolves.toEqual([ + { address: '93.184.216.34', family: 4 }, + ]); + }); + + it.each([ + [ + 'one private record among public ones', + [ + { address: '93.184.216.34', family: 4 }, + { address: '10.0.0.1', family: 4 }, + ], + ], + ['an IPv6 loopback record', [{ address: '::1', family: 6 }]], + [ + 'an IPv4-mapped private record', + [{ address: '::ffff:192.168.1.1', family: 6 }], + ], + ['a NAT64 loopback record', [{ address: '64:ff9b::7f00:1', family: 6 }]], + ])('blocks %s', async (_name, addresses) => { + mockLookup(null, addresses); + await expect(lookup(validators, 'example.com', true)).rejects.toThrow( + validators.WebhookUrlValidationError, + ); + }); + + it('passes through a DNS error unchanged', async () => { + const dnsError = Object.assign(new Error('getaddrinfo ENOTFOUND'), { + code: 'ENOTFOUND', + }); + mockLookup(dnsError, []); + await expect(lookup(validators, 'nope.example')).rejects.toBe(dnsError); + }); + + it('allows an allowlisted hostname that resolves privately', async () => { + const isolatedValidators = loadValidatorsWithConfig({ + hostnameAllowlist: 'localhost', + }); + await expect(lookup(isolatedValidators, 'localhost')).resolves.toEqual( + expect.any(String), + ); + }); + }); + describe('getWebhookConfigErrors', () => { const BROKEN_BODY = '{"text": "{{#if title}}"}'; diff --git a/packages/api/src/utils/validators.ts b/packages/api/src/utils/validators.ts index 14e09f41c0..57a72255c5 100644 --- a/packages/api/src/utils/validators.ts +++ b/packages/api/src/utils/validators.ts @@ -6,7 +6,9 @@ import { PASSWORD_SPECIAL_CHARS, passwordValidators, } from '@hyperdx/common-utils/dist/validation'; +import dns from 'dns'; import { Address4, Address6 } from 'ip-address'; +import type { LookupFunction } from 'net'; import { z } from 'zod'; import * as config from '@/config'; @@ -32,6 +34,8 @@ const IPV4_COMPATIBLE_SUBNET = new Address6('::/96'); const IPV4_MAPPED_SUBNET = new Address6('::ffff:0:0/96'); const IPV6_LINK_LOCAL_SUBNET = new Address6('fe80::/10'); const IPV6_DISCARD_ONLY_SUBNET = new Address6('100::/64'); +const NAT64_WELL_KNOWN_SUBNET = new Address6('64:ff9b::/96'); // RFC 6052 +const NAT64_LOCAL_USE_SUBNET = new Address6('64:ff9b:1::/48'); // RFC 8215 // Returns true for private/reserved IPv4 and IPv6 addresses that must not be // reachable from outbound requests (SSRF protection). @@ -50,10 +54,12 @@ export function isPrivateIp(ip: string): boolean { } if (Address6.isValid(ip)) { const addr = new Address6(ip); - // Check the embedded address in mapped and deprecated compatible forms. + // Check the embedded address in mapped, deprecated compatible, and NAT64 + // forms; a NAT64 gateway forwards 64:ff9b::7f00:1 to 127.0.0.1. if ( addr.isInSubnet(IPV4_MAPPED_SUBNET) || - addr.isInSubnet(IPV4_COMPATIBLE_SUBNET) + addr.isInSubnet(IPV4_COMPATIBLE_SUBNET) || + addr.isInSubnet(NAT64_WELL_KNOWN_SUBNET) ) { const v4 = addr.to4(); if (v4 && Address4.isValid(v4.address)) { @@ -68,7 +74,9 @@ export function isPrivateIp(ip: string): boolean { addr.isMulticast() || addr.isUnspecified() || addr.isDocumentation() || - addr.isInSubnet(IPV6_DISCARD_ONLY_SUBNET) + addr.isInSubnet(IPV6_DISCARD_ONLY_SUBNET) || + // Operator-defined prefixes embed the IPv4 at varying offsets; block all. + addr.isInSubnet(NAT64_LOCAL_USE_SUBNET) ); } return false; @@ -114,6 +122,9 @@ const BLOCKED_WEBHOOK_HOSTS = (() => { })(); export class WebhookUrlValidationError extends Error { + // A 4xx status makes withRetry give up instead of retrying a blocked URL. + readonly status = 400; + constructor(message: string) { super(message); this.name = 'WebhookUrlValidationError'; @@ -163,12 +174,37 @@ export function validateWebhookUrl( hostname === 'localhost' || hostname.endsWith('.localhost'); const isAllowlistedHostname = isAllowlistedWebhookHostname(hostname); if (!isAllowlistedHostname && (isLocalhost || isPrivateIp(hostname))) { - throw new WebhookUrlValidationError( - `Webhook URL resolves to a private or reserved address.`, - ); + throw new WebhookUrlValidationError(PRIVATE_ADDRESS_MESSAGE); } } +const PRIVATE_ADDRESS_MESSAGE = + 'Webhook URL resolves to a private or reserved address.'; + +// validateWebhookUrl only sees the hostname, so a public name pointing at a +// private IP (e.g. 127-0-0-1.nip.io) gets through it. Checking at connect time +// also covers DNS rebinding, since the checked address is the one dialed. +// Node skips lookup for IP literals; validateWebhookUrl covers those. +export const webhookLookup: LookupFunction = (hostname, options, callback) => { + dns.lookup(hostname, { ...options, all: true }, (err, addresses) => { + if (err) return callback(err, []); + const blocked = + !isAllowlistedWebhookHostname(normalizeWebhookHostname(hostname)) && + addresses.some( + ({ address }) => + isPrivateIp(address) && !isAllowlistedWebhookHostname(address), + ); + if (blocked) { + return callback( + new WebhookUrlValidationError(PRIVATE_ADDRESS_MESSAGE), + [], + ); + } + if (options.all) return callback(null, addresses); + callback(null, addresses[0].address, addresses[0].family); + }); +}; + /** A 400-worthy webhook config error whose message is safe to return. */ export const isWebhookValidationError = ( err: unknown, diff --git a/yarn.lock b/yarn.lock index 0d4d19959e..238fed6fed 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4817,6 +4817,7 @@ __metadata: tsc-alias: "npm:^1.8.8" tsconfig-paths: "npm:^4.2.0" typescript: "npm:^6.0.3" + undici: "npm:^7.29.0" uuid: "npm:^8.3.2" zod: "npm:3.25" zod-express-middleware: "npm:^1.4.0"