fix(backend): close two SSRF bypasses in webhook URL validation
1. The URL guard only classified IP-literal hostnames; any plain domain name (the common case) skipped validation entirely. Hostnames are now resolved via DNS and every returned address is checked against the same private/loopback/link-local ranges (net.BlockList), so an attacker-controlled domain pointed at an internal IP is rejected too. 2. fetch() follows redirects by default, so a registered public URL could 302 to an internal target at delivery time. Delivery now uses redirect: 'manual' so such a redirect is never followed.
This commit is contained in:
1 parent
a0f7573af5
commit
79010a926b
4 files changed
+157
-71
No files matched your search
@@ -1,35 +1,47 @@
|
|||||||
import { BadRequestException } from '@nestjs/common';
|
import { BadRequestException } from '@nestjs/common';
|
||||||
import { assertPublicWebhookUrl } from './webhook-url-guard';
|
import { assertPublicWebhookUrl } from './webhook-url-guard';
|
||||||
|
|
||||||
|
function lookupResolving(...addresses: string[]) {
|
||||||
|
return () =>
|
||||||
|
Promise.resolve(addresses.map((address) => ({ address, family: 4 })));
|
||||||
|
}
|
||||||
|
|
||||||
|
function lookupFailing() {
|
||||||
|
return () => Promise.reject(new Error('ENOTFOUND'));
|
||||||
|
}
|
||||||
|
|
||||||
describe('assertPublicWebhookUrl', () => {
|
describe('assertPublicWebhookUrl', () => {
|
||||||
it('accepts a normal public https URL', () => {
|
it('accepts a hostname that resolves only to public addresses', async () => {
|
||||||
expect(() =>
|
await expect(
|
||||||
assertPublicWebhookUrl('https://example.com/hook'),
|
assertPublicWebhookUrl(
|
||||||
).not.toThrow();
|
'https://example.com/hook',
|
||||||
|
lookupResolving('93.184.216.34'),
|
||||||
|
),
|
||||||
|
).resolves.toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('rejects an invalid URL', () => {
|
it('rejects an invalid URL', async () => {
|
||||||
expect(() => assertPublicWebhookUrl('not-a-url')).toThrow(
|
await expect(assertPublicWebhookUrl('not-a-url')).rejects.toBeInstanceOf(
|
||||||
BadRequestException,
|
BadRequestException,
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('rejects non-http(s) schemes', () => {
|
it('rejects non-http(s) schemes', async () => {
|
||||||
expect(() => assertPublicWebhookUrl('ftp://example.com/hook')).toThrow(
|
await expect(
|
||||||
BadRequestException,
|
assertPublicWebhookUrl('ftp://example.com/hook'),
|
||||||
);
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
expect(() => assertPublicWebhookUrl('file:///etc/passwd')).toThrow(
|
await expect(
|
||||||
BadRequestException,
|
assertPublicWebhookUrl('file:///etc/passwd'),
|
||||||
);
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('rejects localhost by hostname', () => {
|
it('rejects localhost by hostname', async () => {
|
||||||
expect(() => assertPublicWebhookUrl('http://localhost/hook')).toThrow(
|
await expect(
|
||||||
BadRequestException,
|
assertPublicWebhookUrl('http://localhost/hook'),
|
||||||
);
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('rejects IPv4 loopback and its shorthand/decimal/hex/octal encodings', () => {
|
it('rejects IPv4 loopback and its shorthand/decimal/hex/octal encodings', async () => {
|
||||||
for (const url of [
|
for (const url of [
|
||||||
'http://127.0.0.1/hook',
|
'http://127.0.0.1/hook',
|
||||||
'http://127.1/hook',
|
'http://127.1/hook',
|
||||||
@@ -37,37 +49,79 @@ describe('assertPublicWebhookUrl', () => {
|
|||||||
'http://2130706433/hook',
|
'http://2130706433/hook',
|
||||||
'http://017700000001/hook',
|
'http://017700000001/hook',
|
||||||
]) {
|
]) {
|
||||||
expect(() => assertPublicWebhookUrl(url)).toThrow(BadRequestException);
|
await expect(assertPublicWebhookUrl(url)).rejects.toBeInstanceOf(
|
||||||
|
BadRequestException,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it('rejects RFC 1918 private ranges', () => {
|
it('rejects RFC 1918 private ranges', async () => {
|
||||||
for (const url of [
|
for (const url of [
|
||||||
'http://10.0.0.5/hook',
|
'http://10.0.0.5/hook',
|
||||||
'http://172.16.0.5/hook',
|
'http://172.16.0.5/hook',
|
||||||
'http://172.31.255.255/hook',
|
'http://172.31.255.255/hook',
|
||||||
'http://192.168.1.1/hook',
|
'http://192.168.1.1/hook',
|
||||||
]) {
|
]) {
|
||||||
expect(() => assertPublicWebhookUrl(url)).toThrow(BadRequestException);
|
await expect(assertPublicWebhookUrl(url)).rejects.toBeInstanceOf(
|
||||||
|
BadRequestException,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it('rejects link-local addresses including the cloud metadata endpoint', () => {
|
it('rejects link-local addresses including the cloud metadata endpoint', async () => {
|
||||||
expect(() =>
|
await expect(
|
||||||
assertPublicWebhookUrl('http://169.254.169.254/latest/meta-data/'),
|
assertPublicWebhookUrl('http://169.254.169.254/latest/meta-data/'),
|
||||||
).toThrow(BadRequestException);
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('does not reject a public IPv4 literal outside reserved ranges', () => {
|
it('does not reject a public IPv4 literal outside reserved ranges', async () => {
|
||||||
expect(() => assertPublicWebhookUrl('http://8.8.8.8/hook')).not.toThrow();
|
await expect(
|
||||||
|
assertPublicWebhookUrl('http://8.8.8.8/hook'),
|
||||||
|
).resolves.toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('rejects IPv6 loopback and link-local addresses', () => {
|
it('rejects IPv6 loopback and link-local addresses', async () => {
|
||||||
expect(() => assertPublicWebhookUrl('http://[::1]/hook')).toThrow(
|
await expect(
|
||||||
BadRequestException,
|
assertPublicWebhookUrl('http://[::1]/hook'),
|
||||||
);
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
expect(() => assertPublicWebhookUrl('http://[fe80::1]/hook')).toThrow(
|
await expect(
|
||||||
BadRequestException,
|
assertPublicWebhookUrl('http://[fe80::1]/hook'),
|
||||||
);
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a hostname that resolves to a private address (DNS-based bypass)', async () => {
|
||||||
|
await expect(
|
||||||
|
assertPublicWebhookUrl(
|
||||||
|
'https://attacker.example/hook',
|
||||||
|
lookupResolving('10.0.0.5'),
|
||||||
|
),
|
||||||
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a hostname that resolves to the cloud metadata address', async () => {
|
||||||
|
await expect(
|
||||||
|
assertPublicWebhookUrl(
|
||||||
|
'https://attacker.example/hook',
|
||||||
|
lookupResolving('169.254.169.254'),
|
||||||
|
),
|
||||||
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a hostname where any resolved address is private, even if others are public', async () => {
|
||||||
|
await expect(
|
||||||
|
assertPublicWebhookUrl(
|
||||||
|
'https://attacker.example/hook',
|
||||||
|
lookupResolving('93.184.216.34', '10.0.0.5'),
|
||||||
|
),
|
||||||
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a hostname that fails to resolve', async () => {
|
||||||
|
await expect(
|
||||||
|
assertPublicWebhookUrl(
|
||||||
|
'https://does-not-exist.invalid/hook',
|
||||||
|
lookupFailing(),
|
||||||
|
),
|
||||||
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -1,46 +1,56 @@
|
|||||||
import { BadRequestException } from '@nestjs/common';
|
import { BadRequestException } from '@nestjs/common';
|
||||||
|
import * as dns from 'node:dns/promises';
|
||||||
import * as net from 'node:net';
|
import * as net from 'node:net';
|
||||||
|
|
||||||
const BLOCKED_HOSTNAMES = new Set(['localhost', '0.0.0.0']);
|
const BLOCKED_HOSTNAMES = new Set(['localhost']);
|
||||||
|
|
||||||
function isPrivateOrReservedIpv4(octets: number[]): boolean {
|
const blockedRanges = new net.BlockList();
|
||||||
const [a, b] = octets;
|
blockedRanges.addSubnet('10.0.0.0', 8, 'ipv4'); // RFC 1918
|
||||||
if (a === 127) return true; // loopback
|
blockedRanges.addSubnet('172.16.0.0', 12, 'ipv4'); // RFC 1918
|
||||||
if (a === 10) return true; // private (RFC 1918)
|
blockedRanges.addSubnet('192.168.0.0', 16, 'ipv4'); // RFC 1918
|
||||||
if (a === 172 && b >= 16 && b <= 31) return true; // private (RFC 1918)
|
blockedRanges.addSubnet('127.0.0.0', 8, 'ipv4'); // loopback
|
||||||
if (a === 192 && b === 168) return true; // private (RFC 1918)
|
blockedRanges.addSubnet('169.254.0.0', 16, 'ipv4'); // link-local, incl. cloud metadata
|
||||||
if (a === 169 && b === 254) return true; // link-local, incl. cloud metadata (169.254.169.254)
|
blockedRanges.addSubnet('0.0.0.0', 8, 'ipv4'); // "this network"
|
||||||
if (a === 0) return true; // "this network"
|
blockedRanges.addAddress('::1', 'ipv6'); // loopback
|
||||||
|
blockedRanges.addAddress('::', 'ipv6'); // unspecified
|
||||||
|
blockedRanges.addSubnet('fe80::', 10, 'ipv6'); // link-local
|
||||||
|
blockedRanges.addSubnet('fc00::', 7, 'ipv6'); // unique local
|
||||||
|
|
||||||
|
function isBlockedIp(ip: string): boolean {
|
||||||
|
const version = net.isIP(ip);
|
||||||
|
if (version === 4) return blockedRanges.check(ip, 'ipv4');
|
||||||
|
if (version === 6) return blockedRanges.check(ip, 'ipv6');
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
function isPrivateOrReservedIp(ip: string): boolean {
|
interface LookupAddress {
|
||||||
if (net.isIPv4(ip)) {
|
address: string;
|
||||||
return isPrivateOrReservedIpv4(ip.split('.').map(Number));
|
family: number;
|
||||||
}
|
|
||||||
if (net.isIPv6(ip)) {
|
|
||||||
const normalized = ip.toLowerCase();
|
|
||||||
if (normalized === '::1' || normalized === '::') return true; // loopback / unspecified
|
|
||||||
if (normalized.startsWith('fe80:')) return true; // link-local
|
|
||||||
if (normalized.startsWith('fc') || normalized.startsWith('fd')) return true; // unique local
|
|
||||||
const mapped = /^::ffff:(\d+\.\d+\.\d+\.\d+)$/.exec(normalized);
|
|
||||||
if (mapped && net.isIPv4(mapped[1])) {
|
|
||||||
return isPrivateOrReservedIpv4(mapped[1].split('.').map(Number));
|
|
||||||
}
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
return false;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type LookupFn = (hostname: string) => Promise<LookupAddress[]>;
|
||||||
|
|
||||||
|
const defaultLookup: LookupFn = (hostname) =>
|
||||||
|
dns.lookup(hostname, { all: true });
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Blocks webhook targets that would let an admin turn the backend into an
|
* Blocks webhook targets that would let an admin turn the backend into an
|
||||||
* SSRF proxy against internal services or the cloud metadata endpoint.
|
* SSRF proxy against internal services or the cloud metadata endpoint.
|
||||||
* Checks are syntactic only (no DNS lookups), relying on the WHATWG URL
|
*
|
||||||
* parser to normalize decimal/octal/hex IPv4 shorthand into dotted form
|
* IP literals (including decimal/octal/hex shorthand, which the WHATWG URL
|
||||||
* before the range check runs; DNS-rebinding (a hostname resolving to a
|
* parser normalizes before this runs) are checked directly against
|
||||||
* private address only at delivery time) is not covered.
|
* `net.BlockList`. Plain hostnames are resolved via DNS and every returned
|
||||||
|
* address is checked too, so an attacker-controlled domain pointed at an
|
||||||
|
* internal IP is rejected rather than silently passing the literal-only
|
||||||
|
* check. This still does not defend against DNS rebinding (the same
|
||||||
|
* hostname resolving to a different, private address only later, at actual
|
||||||
|
* delivery time) — closing that would require re-resolving on every
|
||||||
|
* delivery, which the caller intentionally does not do (see deliver()).
|
||||||
*/
|
*/
|
||||||
export function assertPublicWebhookUrl(rawUrl: string): void {
|
export async function assertPublicWebhookUrl(
|
||||||
|
rawUrl: string,
|
||||||
|
lookup: LookupFn = defaultLookup,
|
||||||
|
): Promise<void> {
|
||||||
let parsed: URL;
|
let parsed: URL;
|
||||||
try {
|
try {
|
||||||
parsed = new URL(rawUrl);
|
parsed = new URL(rawUrl);
|
||||||
@@ -54,13 +64,30 @@ export function assertPublicWebhookUrl(rawUrl: string): void {
|
|||||||
|
|
||||||
const hostname = parsed.hostname.toLowerCase().replace(/^\[|\]$/g, '');
|
const hostname = parsed.hostname.toLowerCase().replace(/^\[|\]$/g, '');
|
||||||
if (BLOCKED_HOSTNAMES.has(hostname)) {
|
if (BLOCKED_HOSTNAMES.has(hostname)) {
|
||||||
throw new BadRequestException(
|
throw new BadRequestException('Webhook URL must not target localhost');
|
||||||
'Webhook URL must not target localhost or a reserved address',
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
if (net.isIP(hostname) && isPrivateOrReservedIp(hostname)) {
|
|
||||||
|
if (net.isIP(hostname)) {
|
||||||
|
if (isBlockedIp(hostname)) {
|
||||||
|
throw new BadRequestException(
|
||||||
|
'Webhook URL must not target a private, loopback, or link-local address',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
let addresses: LookupAddress[];
|
||||||
|
try {
|
||||||
|
addresses = await lookup(hostname);
|
||||||
|
} catch {
|
||||||
|
throw new BadRequestException('Webhook URL host could not be resolved');
|
||||||
|
}
|
||||||
|
if (
|
||||||
|
addresses.length === 0 ||
|
||||||
|
addresses.some((record) => isBlockedIp(record.address))
|
||||||
|
) {
|
||||||
throw new BadRequestException(
|
throw new BadRequestException(
|
||||||
'Webhook URL must not target a private, loopback, or link-local address',
|
'Webhook URL must not resolve to a private, loopback, or link-local address',
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -45,7 +45,7 @@ describe('WebhooksService', () => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
const subscription = await service.create(
|
const subscription = await service.create(
|
||||||
{ url: 'https://example.test/hook', events: ['colony.created'] },
|
{ url: 'https://203.0.113.10/hook', events: ['colony.created'] },
|
||||||
'user-1',
|
'user-1',
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -53,7 +53,7 @@ describe('WebhooksService', () => {
|
|||||||
const [[createArg]] = prisma.webhookSubscription.create.mock.calls as [
|
const [[createArg]] = prisma.webhookSubscription.create.mock.calls as [
|
||||||
[{ data: { url: string; events: string[]; createdByUserId: string } }],
|
[{ data: { url: string; events: string[]; createdByUserId: string } }],
|
||||||
];
|
];
|
||||||
expect(createArg.data.url).toBe('https://example.test/hook');
|
expect(createArg.data.url).toBe('https://203.0.113.10/hook');
|
||||||
expect(createArg.data.events).toEqual(['colony.created']);
|
expect(createArg.data.events).toEqual(['colony.created']);
|
||||||
expect(createArg.data.createdByUserId).toBe('user-1');
|
expect(createArg.data.createdByUserId).toBe('user-1');
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -37,7 +37,7 @@ export class WebhooksService {
|
|||||||
dto: CreateWebhookDto,
|
dto: CreateWebhookDto,
|
||||||
actingUserId: string,
|
actingUserId: string,
|
||||||
): Promise<WebhookSubscription> {
|
): Promise<WebhookSubscription> {
|
||||||
assertPublicWebhookUrl(dto.url);
|
await assertPublicWebhookUrl(dto.url);
|
||||||
const secret = crypto.randomBytes(SECRET_BYTES).toString('hex');
|
const secret = crypto.randomBytes(SECRET_BYTES).toString('hex');
|
||||||
const subscription = await this.prisma.webhookSubscription.create({
|
const subscription = await this.prisma.webhookSubscription.create({
|
||||||
data: {
|
data: {
|
||||||
@@ -104,6 +104,11 @@ export class WebhooksService {
|
|||||||
'X-ApiCore-Signature': `sha256=${signature}`,
|
'X-ApiCore-Signature': `sha256=${signature}`,
|
||||||
},
|
},
|
||||||
body,
|
body,
|
||||||
|
// A redirect target is unvalidated (assertPublicWebhookUrl only
|
||||||
|
// checked the registered URL), so it must not be auto-followed —
|
||||||
|
// otherwise a subscription pointed at an attacker-controlled public
|
||||||
|
// URL could redirect delivery into internal infrastructure.
|
||||||
|
redirect: 'manual',
|
||||||
});
|
});
|
||||||
} catch (error: unknown) {
|
} catch (error: unknown) {
|
||||||
const message = error instanceof Error ? error.message : String(error);
|
const message = error instanceof Error ? error.message : String(error);
|
||||||
|
|||||||
Reference in new issue
Block a user