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.

ApiCore-Backend/src/webhooks/webhook-url-guard.spec.ts

@@ -1,35 +1,47 @@
 import { BadRequestException } from '@nestjs/common';
 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', () => {
-  it('accepts a normal public https URL', () => {
-    expect(() =>
-      assertPublicWebhookUrl('https://example.com/hook'),
-    ).not.toThrow();
+  it('accepts a hostname that resolves only to public addresses', async () => {
+    await expect(
+      assertPublicWebhookUrl(
+        'https://example.com/hook',
+        lookupResolving('93.184.216.34'),
+      ),
+    ).resolves.toBeUndefined();
   });
 
-  it('rejects an invalid URL', () => {
-    expect(() => assertPublicWebhookUrl('not-a-url')).toThrow(
+  it('rejects an invalid URL', async () => {
+    await expect(assertPublicWebhookUrl('not-a-url')).rejects.toBeInstanceOf(
       BadRequestException,
     );
   });
 
-  it('rejects non-http(s) schemes', () => {
-    expect(() => assertPublicWebhookUrl('ftp://example.com/hook')).toThrow(
-      BadRequestException,
-    );
-    expect(() => assertPublicWebhookUrl('file:///etc/passwd')).toThrow(
-      BadRequestException,
-    );
+  it('rejects non-http(s) schemes', async () => {
+    await expect(
+      assertPublicWebhookUrl('ftp://example.com/hook'),
+    ).rejects.toBeInstanceOf(BadRequestException);
+    await expect(
+      assertPublicWebhookUrl('file:///etc/passwd'),
+    ).rejects.toBeInstanceOf(BadRequestException);
   });
 
-  it('rejects localhost by hostname', () => {
-    expect(() => assertPublicWebhookUrl('http://localhost/hook')).toThrow(
-      BadRequestException,
-    );
+  it('rejects localhost by hostname', async () => {
+    await expect(
+      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 [
       'http://127.0.0.1/hook',
       'http://127.1/hook',
@@ -37,37 +49,79 @@ describe('assertPublicWebhookUrl', () => {
       'http://2130706433/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 [
       'http://10.0.0.5/hook',
       'http://172.16.0.5/hook',
       'http://172.31.255.255/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', () => {
-    expect(() =>
+  it('rejects link-local addresses including the cloud metadata endpoint', async () => {
+    await expect(
       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', () => {
-    expect(() => assertPublicWebhookUrl('http://8.8.8.8/hook')).not.toThrow();
+  it('does not reject a public IPv4 literal outside reserved ranges', async () => {
+    await expect(
+      assertPublicWebhookUrl('http://8.8.8.8/hook'),
+    ).resolves.toBeUndefined();
   });
 
-  it('rejects IPv6 loopback and link-local addresses', () => {
-    expect(() => assertPublicWebhookUrl('http://[::1]/hook')).toThrow(
-      BadRequestException,
-    );
-    expect(() => assertPublicWebhookUrl('http://[fe80::1]/hook')).toThrow(
-      BadRequestException,
-    );
+  it('rejects IPv6 loopback and link-local addresses', async () => {
+    await expect(
+      assertPublicWebhookUrl('http://[::1]/hook'),
+    ).rejects.toBeInstanceOf(BadRequestException);
+    await expect(
+      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);
   });
 });

ApiCore-Backend/src/webhooks/webhook-url-guard.ts

@@ -1,46 +1,56 @@
 import { BadRequestException } from '@nestjs/common';
+import * as dns from 'node:dns/promises';
 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 [a, b] = octets;
-  if (a === 127) return true; // loopback
-  if (a === 10) return true; // private (RFC 1918)
-  if (a === 172 && b >= 16 && b <= 31) return true; // private (RFC 1918)
-  if (a === 192 && b === 168) return true; // private (RFC 1918)
-  if (a === 169 && b === 254) return true; // link-local, incl. cloud metadata (169.254.169.254)
-  if (a === 0) return true; // "this network"
+const blockedRanges = new net.BlockList();
+blockedRanges.addSubnet('10.0.0.0', 8, 'ipv4'); // RFC 1918
+blockedRanges.addSubnet('172.16.0.0', 12, 'ipv4'); // RFC 1918
+blockedRanges.addSubnet('192.168.0.0', 16, 'ipv4'); // RFC 1918
+blockedRanges.addSubnet('127.0.0.0', 8, 'ipv4'); // loopback
+blockedRanges.addSubnet('169.254.0.0', 16, 'ipv4'); // link-local, incl. cloud metadata
+blockedRanges.addSubnet('0.0.0.0', 8, 'ipv4'); // "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;
 }
 
-function isPrivateOrReservedIp(ip: string): boolean {
-  if (net.isIPv4(ip)) {
-    return isPrivateOrReservedIpv4(ip.split('.').map(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;
+interface LookupAddress {
+  address: string;
+  family: number;
 }
 
+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
  * 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
- * before the range check runs; DNS-rebinding (a hostname resolving to a
- * private address only at delivery time) is not covered.
+ *
+ * IP literals (including decimal/octal/hex shorthand, which the WHATWG URL
+ * parser normalizes before this runs) are checked directly against
+ * `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;
   try {
     parsed = new URL(rawUrl);
@@ -54,13 +64,30 @@ export function assertPublicWebhookUrl(rawUrl: string): void {
 
   const hostname = parsed.hostname.toLowerCase().replace(/^\[|\]$/g, '');
   if (BLOCKED_HOSTNAMES.has(hostname)) {
-    throw new BadRequestException(
-      'Webhook URL must not target localhost or a reserved address',
-    );
+    throw new BadRequestException('Webhook URL must not target localhost');
   }
-  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(
+      'Webhook URL must not resolve to a private, loopback, or link-local address',
+    );
+  }
 }

ApiCore-Backend/src/webhooks/webhooks.service.spec.ts

@@ -45,7 +45,7 @@ describe('WebhooksService', () => {
     );
 
     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',
     );
 
@@ -53,7 +53,7 @@ describe('WebhooksService', () => {
     const [[createArg]] = prisma.webhookSubscription.create.mock.calls as [
       [{ 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.createdByUserId).toBe('user-1');
   });

ApiCore-Backend/src/webhooks/webhooks.service.ts

@@ -37,7 +37,7 @@ export class WebhooksService {
     dto: CreateWebhookDto,
     actingUserId: string,
   ): Promise<WebhookSubscription> {
-    assertPublicWebhookUrl(dto.url);
+    await assertPublicWebhookUrl(dto.url);
     const secret = crypto.randomBytes(SECRET_BYTES).toString('hex');
     const subscription = await this.prisma.webhookSubscription.create({
       data: {
@@ -104,6 +104,11 @@ export class WebhooksService {
           'X-ApiCore-Signature': `sha256=${signature}`,
         },
         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) {
       const message = error instanceof Error ? error.message : String(error);