fix: resolve DNS before SSRF checks to close rebinding bypass (CWE-918)

src/__tests__/data-layer.test.ts

@@ -27,8 +27,23 @@ vi.mock("../agent/injection-defense.js", () => ({
   sanitizeInput: vi.fn((s: string) => ({ content: s, blocked: false })),
 }));
 
+// Mock DNS resolution so isAllowedUri's rebinding check is deterministic
+// and doesn't require network access in tests. Hostnames not listed here
+// resolve to a public IP by default.
+const dnsMockAddresses: Record<string, string[]> = {
+  "example.com": ["93.184.216.34"],
+  "api.conway.tech": ["203.0.113.10"],
+};
+vi.mock("node:dns/promises", () => ({
+  lookup: vi.fn(async (hostname: string, _opts?: unknown) => {
+    const addresses = dnsMockAddresses[hostname] ?? ["203.0.113.99"];
+    return addresses.map((address) => ({ address, family: 4 }));
+  }),
+}));
+
 // Import after mocks are set up
 const { isAllowedUri, isInternalNetwork, validateAgentCard } = await import("../registry/discovery.js");
+const dns = await import("node:dns/promises");
 const { loadInstalledTools } = await import("../agent/tools.js");
 
 function makeTmpDbPath(): string {
@@ -109,40 +124,79 @@ describe("SSRF Protection", () => {
   });
 
   describe("isAllowedUri", () => {
-    it("allows https URIs", () => {
-      expect(isAllowedUri("https://example.com/agent-card.json")).toBe(true);
+    it("allows https URIs", async () => {
+      expect(await isAllowedUri("https://example.com/agent-card.json")).toBe(true);
+    });
+
+    it("allows ipfs URIs", async () => {
+      expect(await isAllowedUri("ipfs://QmTest123")).toBe(true);
+    });
+
+    it("blocks http URIs", async () => {
+      expect(await isAllowedUri("http://example.com/agent-card.json")).toBe(false);
+    });
+
+    it("blocks file URIs", async () => {
+      expect(await isAllowedUri("file:///etc/passwd")).toBe(false);
+    });
+
+    it("blocks ftp URIs", async () => {
+      expect(await isAllowedUri("ftp://evil.com/data")).toBe(false);
+    });
+
+    it("blocks javascript URIs", async () => {
+      expect(await isAllowedUri("javascript:alert(1)")).toBe(false);
     });
 
-    it("allows ipfs URIs", () => {
-      expect(isAllowedUri("ipfs://QmTest123")).toBe(true);
+    it("blocks https URIs to internal networks", async () => {
+      expect(await isAllowedUri("https://127.0.0.1/card.json")).toBe(false);
+      expect(await isAllowedUri("https://10.0.0.1/card.json")).toBe(false);
+      expect(await isAllowedUri("https://192.168.1.1/card.json")).toBe(false);
+      expect(await isAllowedUri("https://localhost/card.json")).toBe(false);
     });
 
-    it("blocks http URIs", () => {
-      expect(isAllowedUri("http://example.com/agent-card.json")).toBe(false);
+    it("blocks invalid URIs", async () => {
+      expect(await isAllowedUri("not-a-url")).toBe(false);
+      expect(await isAllowedUri("")).toBe(false);
     });
 
-    it("blocks file URIs", () => {
-      expect(isAllowedUri("file:///etc/passwd")).toBe(false);
+    it("blocks DNS-rebinding: a hostname that resolves to a private IP (CWE-918)", async () => {
+      const dnsModule = dns as unknown as {
+        lookup: (hostname: string, opts?: unknown) => Promise<Array<{ address: string; family: number }>>;
+      };
+      vi.mocked(dnsModule.lookup).mockImplementationOnce(async () => [
+        { address: "127.0.0.1", family: 4 },
+      ]);
+      expect(await isAllowedUri("https://evil.attacker.com/card.json")).toBe(false);
     });
 
-    it("blocks ftp URIs", () => {
-      expect(isAllowedUri("ftp://evil.com/data")).toBe(false);
+    it("blocks DNS-rebinding: one of several resolved addresses is private", async () => {
+      const dnsModule = dns as unknown as {
+        lookup: (hostname: string, opts?: unknown) => Promise<Array<{ address: string; family: number }>>;
+      };
+      vi.mocked(dnsModule.lookup).mockImplementationOnce(async () => [
+        { address: "203.0.113.5", family: 4 },
+        { address: "10.0.0.1", family: 4 },
+      ]);
+      expect(await isAllowedUri("https://multi-a-record.example/card.json")).toBe(false);
     });
 
-    it("blocks javascript URIs", () => {
-      expect(isAllowedUri("javascript:alert(1)")).toBe(false);
+    it("fails closed when DNS resolution errors", async () => {
+      const dnsModule = dns as unknown as {
+        lookup: (hostname: string, opts?: unknown) => Promise<Array<{ address: string; family: number }>>;
+      };
+      vi.mocked(dnsModule.lookup).mockImplementationOnce(async () => {
+        throw new Error("ENOTFOUND");
+      });
+      expect(await isAllowedUri("https://does-not-resolve.example/card.json")).toBe(false);
     });
 
-    it("blocks https URIs to internal networks", () => {
-      expect(isAllowedUri("https://127.0.0.1/card.json")).toBe(false);
-      expect(isAllowedUri("https://10.0.0.1/card.json")).toBe(false);
-      expect(isAllowedUri("https://192.168.1.1/card.json")).toBe(false);
-      expect(isAllowedUri("https://localhost/card.json")).toBe(false);
+    it("blocks IPv6-mapped IPv4 loopback (::ffff:127.0.0.1)", async () => {
+      expect(await isAllowedUri("https://[::ffff:127.0.0.1]/card.json")).toBe(false);
     });
 
-    it("blocks invalid URIs", () => {
-      expect(isAllowedUri("not-a-url")).toBe(false);
-      expect(isAllowedUri("")).toBe(false);
+    it("blocks IPv6-mapped IPv4 loopback (compressed hex form)", async () => {
+      expect(await isAllowedUri("https://[::ffff:7f00:1]/card.json")).toBe(false);
     });
   });
 });

src/__tests__/discovery-data-uri.test.ts

@@ -23,6 +23,12 @@ vi.mock("../agent/injection-defense.js", () => ({
   sanitizeInput: vi.fn((s: string) => ({ content: s, blocked: false })),
 }));
 
+// Mock DNS resolution so isAllowedUri's rebinding check doesn't require
+// network access in tests; example.com resolves to a public IP.
+vi.mock("node:dns/promises", () => ({
+  lookup: vi.fn(async () => [{ address: "93.184.216.34", family: 4 }]),
+}));
+
 // Import after mocks are set up
 const { fetchAgentCard, isAllowedUri } = await import("../registry/discovery.js");
 
@@ -131,7 +137,7 @@ describe("fetchAgentCard — data: URI handling", () => {
     expect(result).toBeNull();
 
     // Confirm isAllowedUri rejects data:text/html
-    expect(isAllowedUri(uri)).toBe(false);
+    expect(await isAllowedUri(uri)).toBe(false);
   });
 
   it("returns null when validateAgentCard rejects schema", async () => {
@@ -150,6 +156,6 @@ describe("fetchAgentCard — data: URI handling", () => {
     // it doesn't go through the data: path.
     const httpsUri = "https://example.com/agent-card.json";
     expect(httpsUri.startsWith("data:application/json")).toBe(false);
-    expect(isAllowedUri(httpsUri)).toBe(true);
+    expect(await isAllowedUri(httpsUri)).toBe(true);
   });
 });

src/__tests__/social.test.ts

@@ -9,6 +9,12 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import Database from "better-sqlite3";
 import { MIGRATION_V7 } from "../state/schema.js";
 
+// Mock DNS resolution so isAllowedUri's rebinding check doesn't require
+// network access in tests; example.com resolves to a public IP.
+vi.mock("node:dns/promises", () => ({
+  lookup: vi.fn(async () => [{ address: "93.184.216.34", family: 4 }]),
+}));
+
 // ─── Test helpers ───────────────────────────────────────────────
 
 function createTestDb(): import("better-sqlite3").Database {
@@ -608,17 +614,17 @@ describe("Discovery", () => {
 
   it("isAllowedUri blocks HTTP", async () => {
     const { isAllowedUri } = await import("../registry/discovery.js");
-    expect(isAllowedUri("http://example.com/card.json")).toBe(false);
+    expect(await isAllowedUri("http://example.com/card.json")).toBe(false);
   });
 
   it("isAllowedUri allows HTTPS", async () => {
     const { isAllowedUri } = await import("../registry/discovery.js");
-    expect(isAllowedUri("https://example.com/card.json")).toBe(true);
+    expect(await isAllowedUri("https://example.com/card.json")).toBe(true);
   });
 
   it("isAllowedUri blocks localhost", async () => {
     const { isAllowedUri } = await import("../registry/discovery.js");
-    expect(isAllowedUri("https://localhost/card.json")).toBe(false);
+    expect(await isAllowedUri("https://localhost/card.json")).toBe(false);
   });
 });
 

src/registry/discovery.ts

@@ -17,6 +17,7 @@ import { DEFAULT_DISCOVERY_CONFIG } from "../types.js";
 import { queryAgent, getTotalAgents, getRegisteredAgentsByEvents } from "./erc8004.js";
 import { keccak256, toBytes } from "viem";
 import { createLogger } from "../observability/logger.js";
+import { lookup as dnsLookup } from "node:dns/promises";
 const logger = createLogger("registry.discovery");
 
 type Network = "mainnet" | "testnet";
@@ -27,11 +28,36 @@ const DISCOVERY_TIMEOUT_MS = 60_000;
 // ─── SSRF Protection ────────────────────────────────────────────
 
 /**
- * Check if a hostname resolves to an internal/private network.
+ * Unwrap an IPv6-mapped IPv4 address (e.g. "::ffff:127.0.0.1" or the
+ * compressed hex form "::ffff:7f00:1") to its dotted-quad IPv4 form.
+ * Returns the input unchanged if it isn't an IPv4-mapped address.
+ */
+function unwrapIPv4MappedAddress(address: string): string {
+  const stripped = address.replace(/^\[|\]$/g, "");
+  const dotted = stripped.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/i);
+  if (dotted) return dotted[1];
+  const hex = stripped.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/i);
+  if (hex) {
+    const hi = parseInt(hex[1], 16);
+    const lo = parseInt(hex[2], 16);
+    return `${(hi >> 8) & 0xff}.${hi & 0xff}.${(lo >> 8) & 0xff}.${lo & 0xff}`;
+  }
+  return stripped;
+}
+
+/**
+ * Check if a hostname or IP literal is an internal/private network address.
  * Blocks: 127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12,
- *         192.168.0.0/16, 169.254.0.0/16, ::1, localhost, 0.0.0.0/8
+ *         192.168.0.0/16, 169.254.0.0/16, ::1, localhost, 0.0.0.0/8,
+ *         and IPv6-mapped forms of the above.
+ *
+ * This is a literal string check only — it does NOT resolve DNS, so a
+ * hostname that *resolves* to a private IP will not be caught here.
+ * Use `isAllowedUri` (which does resolve DNS) to fully validate a URI
+ * before fetching it.
  */
 export function isInternalNetwork(hostname: string): boolean {
+  const normalized = unwrapIPv4MappedAddress(hostname);
   const blocked = [
     /^127\./,
     /^10\./,
@@ -42,19 +68,41 @@ export function isInternalNetwork(hostname: string): boolean {
     /^localhost$/i,
     /^0\./,
   ];
-  return blocked.some(pattern => pattern.test(hostname));
+  return blocked.some(pattern => pattern.test(normalized));
+}
+
+/**
+ * Resolve `hostname` via DNS and check whether ANY resolved address is
+ * an internal/private network address. Closes the DNS-rebinding gap where
+ * an attacker-controlled domain resolves to a private IP (CWE-918):
+ * checking the hostname string alone is not sufficient.
+ *
+ * Fails closed: if resolution errors (NXDOMAIN, no network, etc.), the
+ * hostname is treated as blocked, since we can't prove it's safe to fetch.
+ */
+async function resolvesToInternalNetwork(hostname: string): Promise<boolean> {
+  if (isInternalNetwork(hostname)) return true;
+  try {
+    const results = await dnsLookup(hostname, { all: true });
+    return results.some((r) => isInternalNetwork(r.address));
+  } catch {
+    return true;
+  }
 }
 
 /**
  * Check if a URI is allowed for fetching.
  * Only https: and ipfs: schemes are permitted.
- * Internal network addresses are blocked (SSRF protection).
+ * Internal network addresses are blocked (SSRF protection), including
+ * hostnames that resolve to a private IP via DNS rebinding.
  */
-export function isAllowedUri(uri: string): boolean {
+export async function isAllowedUri(uri: string): Promise<boolean> {
   try {
     const url = new URL(uri);
     if (!['https:', 'ipfs:'].includes(url.protocol)) return false;
-    if (url.protocol === 'https:' && isInternalNetwork(url.hostname)) return false;
+    if (url.protocol === 'https:' && (await resolvesToInternalNetwork(url.hostname))) {
+      return false;
+    }
     return true;
   } catch {
     return false;
@@ -311,7 +359,7 @@ export async function fetchAgentCard(
   }
 
   // SSRF protection: validate URI before fetching
-  if (!isAllowedUri(uri)) {
+  if (!(await isAllowedUri(uri))) {
     logger.error(`Blocked URI (SSRF protection): ${uri}`);
     return null;
   }