🛡️ Sentinel: [CRITICAL] Fix rate limit DoS vulnerability (#136)

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>

.jules/sentinel.md

@@ -13,3 +13,7 @@
 **Vulnerability:** The rate limiter implemented an LRU/FIFO map to track API request limits, evicting the oldest tracked IP when maximum capacity was reached. An attacker could flood the store with randomly spoofed IP headers, continuously evicting legitimate users' IP addresses. This allowed the attacker to effectively bypass their own rate limit and subject the upstream server to DoS.
 **Learning:** Evicting entries from a security control store (like a rate limit map) under pressure turns the limit into a revolving door if an attacker controls the keys (e.g., via IP spoofing headers).
 **Prevention:** Rather than evicting old entries when an in-memory rate limit store is full, the correct security posture is to fail closed (or block new keys) until entries naturally expire, preserving tracking on existing keys.
+## 2026-04-23 - Prevent Global DoS in Rate Limiter via FIFO Eviction
+**Vulnerability:** The in-memory rate limiter implemented a hard ceiling (`MAX_STORE_ENTRIES`) and rejected all new IPs when reached, creating a trivial Global Denial of Service (DoS) vulnerability where an attacker could spam spoofed IPs to fill the bounded Map and permanently block all legitimate traffic.
+**Learning:** "Failing securely" by denying unknown requests during capacity limits is incorrect for rate limiters. Bounded Maps must actively evict older entries rather than rejecting new ones, otherwise the security mechanism itself becomes the vector for DoS.
+**Prevention:** In-memory rate limiters using bounded Maps must implement a FIFO (First-In-First-Out) or LRU (Least Recently Used) eviction policy (e.g., deleting `store.keys().next().value`) to make room for new IPs when maximum capacity is reached.

app/api/map/route.ts

@@ -10,16 +10,13 @@ import { getConfig } from '@/lib/config';
 import { imageUrl } from '@/lib/urls';
 import { isAuthenticated } from '@/lib/auth';
 import { getMapData } from '@/lib/mapService';
-import { checkRateLimit } from '@/lib/rate-limit';
+import { checkRateLimit, getClientIp } from '@/lib/rate-limit';
 
 export const dynamic = 'force-dynamic';
 
 export async function GET(request: NextRequest) {
   // Rate limit: 120 requests/minute per IP (map data is cached client-side; this guards against abuse)
-  const ip =
+  const ip = getClientIp(request);
-    request.headers.get('x-forwarded-for')?.split(',')[0].trim() ??
-    request.headers.get('x-real-ip') ??
-    '127.0.0.1';
   const rl = checkRateLimit(ip, 120);
   if (!rl.success) {
     return NextResponse.json(

lib/__tests__/rate-limit.test.ts

@@ -103,7 +103,7 @@ describe('checkRateLimit', () => {
     expect(result.resetAt).toBeLessThanOrEqual(before + 61_000);
   });
 
-  it('does not crash when store is at capacity and prevents rate limit bypasses', () => {
+  it('does not crash when store is at capacity and allows new requests by evicting oldest entries', () => {
     // Fill the store beyond MAX_STORE_ENTRIES (10_000 in prod)
     // by hammering with unique IPs.
     const uniqueIps = Array.from({ length: 10005 }, (_, i) => `evict-test-ip-${i}-${Date.now()}`);
@@ -111,10 +111,10 @@ describe('checkRateLimit', () => {
       checkRateLimit(ip, 5);
     }
 
-    // A brand-new IP must be blocked to prevent spoofing-based cache floods
+    // A brand-new IP must be allowed by evicting the oldest entry
     const freshIp = `evict-fresh-${Date.now()}`;
     const result = checkRateLimit(freshIp, 5);
-    expect(result.success).toBe(false);
+    expect(result.success).toBe(true);
-    expect(result.remaining).toBe(0);
+    expect(result.remaining).toBe(4);
   });
 });

lib/rate-limit.ts

@@ -67,10 +67,12 @@ export function checkRateLimit(
   // New window or expired
   if (!entry || now > entry.expiresAt) {
     if (store.size >= MAX_STORE_ENTRIES) {
-      // Store is full. Prevent adding new entries to mitigate cache flooding
+      // Store is full. Evict the oldest entry to prevent cache flooding
-      // where an attacker spams spoofed IPs to evict legitimate users.
+      // DOS attacks, which could otherwise block all legitimate users.
-      // Deny new unrecognized IPs to fail securely during an active flood.
+      const oldestKey = store.keys().next().value;
-      return { success: false, remaining: 0, resetAt: now + windowMs };
+      if (oldestKey !== undefined) {
+        store.delete(oldestKey);
+      }
     }
     const resetAt = now + windowMs;
     store.set(key, { count: 1, expiresAt: resetAt });