Stop keying the rate limiter on spoofable forwarding headers

lib/security/rate-limit.ts

@@ -1,40 +1,87 @@
+import { isIP } from "node:net";
+
 type Bucket = { count: number; resetAt: number };
 
 const WINDOW_MS = 60_000;
 const MAX_PER_WINDOW = 12;
+const GLOBAL_MAX_PER_WINDOW = 120;
+const MAX_BUCKETS = 1_000;
+
+const GLOBAL_KEY = "\u0000global";
 
 const buckets = new Map<string, Bucket>();
 let lastSweepAt = 0;
 
-export function rateLimit(key: string): { ok: true } | { ok: false; retryAfterSec: number } {
-  const now = Date.now();
-  if (now - lastSweepAt >= WINDOW_MS || buckets.size > 1000) {
-    for (const [bucketKey, value] of buckets) {
-      if (value.resetAt <= now) buckets.delete(bucketKey);
-    }
-    lastSweepAt = now;
-  }
+function takeToken(
+  key: string,
+  max: number,
+  now: number,
+): { ok: true } | { ok: false; retryAfterSec: number } {
   const bucket = buckets.get(key);
   if (!bucket || bucket.resetAt <= now) {
+    if (!bucket && buckets.size >= MAX_BUCKETS) evictSoonestExpiring();
     buckets.set(key, { count: 1, resetAt: now + WINDOW_MS });
     return { ok: true };
   }
-  if (bucket.count >= MAX_PER_WINDOW) {
+  if (bucket.count >= max) {
     return { ok: false, retryAfterSec: Math.ceil((bucket.resetAt - now) / 1000) };
   }
   bucket.count += 1;
   return { ok: true };
 }
 
+function evictSoonestExpiring(): void {
+  let victim: string | undefined;
+  let earliest = Infinity;
+  for (const [bucketKey, value] of buckets) {
+    if (bucketKey === GLOBAL_KEY) continue;
+    if (value.resetAt < earliest) {
+      earliest = value.resetAt;
+      victim = bucketKey;
+    }
+  }
+  if (victim !== undefined) buckets.delete(victim);
+}
+
+export function rateLimit(key: string): { ok: true } | { ok: false; retryAfterSec: number } {
+  const now = Date.now();
+  if (now - lastSweepAt >= WINDOW_MS || buckets.size > MAX_BUCKETS) {
+    for (const [bucketKey, value] of buckets) {
+      if (value.resetAt <= now) buckets.delete(bucketKey);
+    }
+    lastSweepAt = now;
+  }
+  const global = takeToken(GLOBAL_KEY, GLOBAL_MAX_PER_WINDOW, now);
+  if (!global.ok) return global;
+  return takeToken(key, MAX_PER_WINDOW, now);
+}
+
+function trustsProxyHeaders(): boolean {
+  return process.env.VERCEL === "1" || process.env.TRUST_PROXY_HEADERS === "1";
+}
+
+function firstValidIp(value: string | null): string | null {
+  if (!value) return null;
+  const first = value.split(",")[0]?.trim();
+  return first && isIP(first) ? first : null;
+}
+
+// Forwarding headers are attacker-controlled unless a trusted proxy overwrites
+// them before the request reaches the handler. They are only honored when the
+// deployment declares such a proxy (Vercel edge, or TRUST_PROXY_HEADERS=1),
+// and only when the value parses as an IP address. Otherwise every request
+// shares a single bucket, and the global limiter bounds total throughput.
 export function clientKeyFromHeaders(headers: Headers): string {
-  const vercel = headers.get("x-vercel-forwarded-for")?.trim();
+  if (!trustsProxyHeaders()) return "direct";
+  const vercel = firstValidIp(headers.get("x-vercel-forwarded-for"));
   if (vercel) return vercel;
-  const realIp = headers.get("x-real-ip")?.trim();
+  const realIp = firstValidIp(headers.get("x-real-ip"));
   if (realIp) return realIp;
   const xff = headers.get("x-forwarded-for");
   if (xff) {
     const entries = xff.split(",").map((value) => value.trim()).filter(Boolean);
-    if (entries.length > 0) return entries[entries.length - 1];
+    const last = entries[entries.length - 1];
+    if (last && isIP(last)) return last;
   }
-  return "unknown";
+  return "direct";
 }

tests/security/rate-limit.test.ts

@@ -1,20 +1,47 @@
-import { describe, expect, it, vi } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 
 describe("rate limiting", () => {
-  it("uses hardened client-key precedence", async () => {
+  afterEach(() => {
+    vi.unstubAllEnvs();
+  });
+
+  it("ignores forwarding headers when no trusted proxy is declared", async () => {
+    vi.resetModules();
+    vi.stubEnv("VERCEL", "");
+    vi.stubEnv("TRUST_PROXY_HEADERS", "");
+    const { clientKeyFromHeaders } = await import("@/lib/security/rate-limit");
+    expect(clientKeyFromHeaders(new Headers({
+      "x-vercel-forwarded-for": "1.2.3.4",
+      "x-real-ip": "5.6.7.8",
+      "x-forwarded-for": "9.9.9.9, 10.10.10.10",
+    }))).toBe("direct");
+    expect(clientKeyFromHeaders(new Headers())).toBe("direct");
+  });
+
+  it("uses hardened client-key precedence behind a trusted proxy", async () => {
     vi.resetModules();
+    vi.stubEnv("TRUST_PROXY_HEADERS", "1");
     const { clientKeyFromHeaders } = await import("@/lib/security/rate-limit");
     expect(clientKeyFromHeaders(new Headers({
-      "x-vercel-forwarded-for": "vercel-client",
-      "x-real-ip": "real-client",
-      "x-forwarded-for": "left, right",
-    }))).toBe("vercel-client");
+      "x-vercel-forwarded-for": "1.2.3.4",
+      "x-real-ip": "5.6.7.8",
+      "x-forwarded-for": "9.9.9.9, 10.10.10.10",
+    }))).toBe("1.2.3.4");
     expect(clientKeyFromHeaders(new Headers({
-      "x-real-ip": "real-client",
-      "x-forwarded-for": "left, right",
-    }))).toBe("real-client");
-    expect(clientKeyFromHeaders(new Headers({ "x-forwarded-for": "left, right" }))).toBe("right");
-    expect(clientKeyFromHeaders(new Headers())).toBe("unknown");
+      "x-real-ip": "5.6.7.8",
+      "x-forwarded-for": "9.9.9.9, 10.10.10.10",
+    }))).toBe("5.6.7.8");
+    expect(clientKeyFromHeaders(new Headers({ "x-forwarded-for": "9.9.9.9, 10.10.10.10" }))).toBe("10.10.10.10");
+    expect(clientKeyFromHeaders(new Headers())).toBe("direct");
+  });
+
+  it("rejects header values that are not IP addresses", async () => {
+    vi.resetModules();
+    vi.stubEnv("TRUST_PROXY_HEADERS", "1");
+    const { clientKeyFromHeaders } = await import("@/lib/security/rate-limit");
+    expect(clientKeyFromHeaders(new Headers({ "x-vercel-forwarded-for": "spoofed-token" }))).toBe("direct");
+    expect(clientKeyFromHeaders(new Headers({ "x-real-ip": "not.an.ip" }))).toBe("direct");
+    expect(clientKeyFromHeaders(new Headers({ "x-forwarded-for": "abc, def" }))).toBe("direct");
   });
 
   it("trips after twelve requests with a useful retry time", async () => {
@@ -25,4 +52,19 @@ describe("rate limiting", () => {
     expect(blocked.ok).toBe(false);
     if (!blocked.ok) expect(blocked.retryAfterSec).toBeGreaterThanOrEqual(1);
   });
+
+  it("enforces a global cap independent of the per-client key", async () => {
+    vi.resetModules();
+    const { rateLimit } = await import("@/lib/security/rate-limit");
+    let blockedAt = -1;
+    for (let i = 0; i < 200; i++) {
+      const result = rateLimit(`rotating-${i}`);
+      if (!result.ok) {
+        blockedAt = i;
+        expect(result.retryAfterSec).toBeGreaterThanOrEqual(1);
+        break;
+      }
+    }
+    expect(blockedAt).toBe(120);
+  });
 });