From 32280b9cc10e5366bc2af9a5fee9934015987ece Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 26 Sep 2026 15:07:09 +0200 Subject: [PATCH] fix: cap admin sign-in attempts across all clients (GHSA-7pj2-232x-6698) The admin login limit keys on the client IP taken from X-Forwarded-For. With no reverse proxy in front, that header is whatever the client sends (Next only fills it from the socket when the request carries none), so each forged address got a fresh bucket of five guesses. Route handlers cannot see the socket address, so the limiter now also stops at 50 attempts per 15 minutes across all clients. `.env.example` says why a proxy that appends the header is still needed for trustworthy per-IP limits and audit addresses. --- .env.example | 4 +++ lib/__tests__/admin-rate-limit.test.ts | 44 ++++++++++++++++++++++++++ lib/admin/rate-limit.ts | 23 +++++++++++++- 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 lib/__tests__/admin-rate-limit.test.ts diff --git a/.env.example b/.env.example index 2f9d4299f..14a59e603 100644 --- a/.env.example +++ b/.env.example @@ -370,6 +370,10 @@ LOGIN_WEBSITE_URL=https://bulwarkmail.org # How many trusted reverse proxies sit in front of the app. The client IP is # taken that many entries from the right of X-Forwarded-For, so an attacker # can't spoof it by prepending values. Default: 1. +# Without any reverse proxy the header is whatever the client sends, so +# per-IP limits and audit-log addresses can be forged; admin sign-in then +# still stops at 50 attempts per 15 minutes across all clients. Put the +# app behind a proxy that appends X-Forwarded-For. # TRUSTED_PROXY_DEPTH=2 # Allow search engines to index the app (robots.txt / noindex). Default: false. diff --git a/lib/__tests__/admin-rate-limit.test.ts b/lib/__tests__/admin-rate-limit.test.ts new file mode 100644 index 000000000..6699b494a --- /dev/null +++ b/lib/__tests__/admin-rate-limit.test.ts @@ -0,0 +1,44 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// The per-IP key is X-Forwarded-For, which a client writes itself when the +// app is reachable without a reverse proxy: a fresh forged address used to +// mean a fresh bucket of five guesses, without end. + +async function load() { + vi.resetModules(); + return import('@/lib/admin/rate-limit'); +} + +describe('admin login rate limit', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('allows five attempts per address', async () => { + const { checkRateLimit } = await load(); + for (let i = 0; i < 5; i++) expect(checkRateLimit('203.0.113.1').allowed).toBe(true); + expect(checkRateLimit('203.0.113.1').allowed).toBe(false); + expect(checkRateLimit('203.0.113.2').allowed).toBe(true); + }); + + it('stops rotating forged addresses at the global ceiling', async () => { + const { checkRateLimit } = await load(); + let allowed = 0; + for (let i = 0; i < 200; i++) { + if (checkRateLimit(`198.51.100.${i}`).allowed) allowed++; + } + expect(allowed).toBe(50); + expect(checkRateLimit('192.0.2.77').allowed).toBe(false); + }); + + it('opens again after the window', async () => { + const { checkRateLimit } = await load(); + for (let i = 0; i < 60; i++) checkRateLimit(`198.51.100.${i}`); + vi.advanceTimersByTime(15 * 60 * 1000 + 1); + expect(checkRateLimit('192.0.2.77').allowed).toBe(true); + }); +}); diff --git a/lib/admin/rate-limit.ts b/lib/admin/rate-limit.ts index 49fea10b6..c4b13930a 100644 --- a/lib/admin/rate-limit.ts +++ b/lib/admin/rate-limit.ts @@ -1,17 +1,28 @@ /** * In-memory rate limiter for admin login. - * Max 5 attempts per IP per 15 minutes. + * Max 5 attempts per IP per 15 minutes, and 50 across all clients. */ const MAX_ATTEMPTS = 5; const WINDOW_MS = 15 * 60 * 1000; // 15 minutes +/** + * Ceiling across all clients. The per-IP key comes from X-Forwarded-For + * (getClientIP), which a client can write itself when the app is reachable + * without a reverse proxy in front: Next fills the header from the socket + * only when the request carries none. Rotating forged addresses would + * otherwise buy unlimited guesses. 50 per window still leaves room for a + * few admins mistyping at once. + */ +const GLOBAL_MAX_ATTEMPTS = 50; + interface RateLimitEntry { count: number; resetAt: number; } const attempts = new Map(); +let globalAttempts: RateLimitEntry | null = null; // Clean up expired entries periodically setInterval(() => { @@ -28,11 +39,20 @@ setInterval(() => { */ export function checkRateLimit(ip: string): { allowed: boolean; remaining: number; retryAfterMs: number } { const now = Date.now(); + + if (!globalAttempts || globalAttempts.resetAt <= now) { + globalAttempts = { count: 0, resetAt: now + WINDOW_MS }; + } + if (globalAttempts.count >= GLOBAL_MAX_ATTEMPTS) { + return { allowed: false, remaining: 0, retryAfterMs: globalAttempts.resetAt - now }; + } + const entry = attempts.get(ip); if (!entry || entry.resetAt <= now) { // New window attempts.set(ip, { count: 1, resetAt: now + WINDOW_MS }); + globalAttempts.count++; return { allowed: true, remaining: MAX_ATTEMPTS - 1, retryAfterMs: 0 }; } @@ -41,5 +61,6 @@ export function checkRateLimit(ip: string): { allowed: boolean; remaining: numbe } entry.count++; + globalAttempts.count++; return { allowed: true, remaining: MAX_ATTEMPTS - entry.count, retryAfterMs: 0 }; }