diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 29abdfb..8d56fc8 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -4,3 +4,8 @@ **Vulnerability:** The application used `scryptSync` (synchronous CPU-intensive password hashing) inside Next.js server action handlers for registration and login. Because Node.js runs on a single main event loop, a small number of concurrent authentication requests (or a distributed credential stuffing attack) completely blocks the event loop, starving all other concurrent requests and causing a full Denial of Service (DoS). **Learning:** Next.js Server Actions and Route Handlers run on Node's main thread by default. Using synchronous cryptography operations (such as `scryptSync` or `pbkdf2Sync`) prevents the server from processing other concurrent connections. **Prevention:** Always use asynchronous password-hashing implementations (such as async `scrypt` wrapped in a Promise or bcrypt/argon2 async variants) inside Next.js/Node.js web entry points to delegate heavy hashing computations to the Node.js libuv thread pool, keeping the main event loop responsive. + +## 2026-07-19 - Single-Key Identifier Rate Limiting Enables Account Lockout DoS +**Vulnerability:** Authentication server actions (`signUpAction`, `signInAction`) previously rate-limited requests solely by target email (`signup:email` or `signin:email`). An attacker could trigger lockout for target emails without affecting their own capacity, or launch distributed credential stuffing from a single IP address across many accounts. +**Learning:** Single-key rate limiting indexed by target identifier creates an asymmetric attack vector where attackers cause Denial of Service against victim accounts (Account Lockout DoS). +**Prevention:** Use dual rate limiting (`rateLimitDual`) on sensitive authentication actions: evaluate client IP rate limits first before checking target identifier limits. Checking IP limits first prevents blocked attacker IPs from consuming or polluting target-based rate limit buckets. diff --git a/package.json b/package.json index e60c440..2898b36 100644 --- a/package.json +++ b/package.json @@ -72,5 +72,5 @@ "eslint --fix" ] }, - "packageManager": "pnpm@11.13.0+sha512.88d94724d8f2e6c186744a5584c6e59ecac869ec7ba15e9cb4cd628e8dc7066820b2481d8ee3b51ea8da323a7378068aa58c556a3720d32b7c20a051d088363a" + "packageManager": "pnpm@11.14.0" } diff --git a/src/app/actions/auth.ts b/src/app/actions/auth.ts index 2f10075..748582d 100644 --- a/src/app/actions/auth.ts +++ b/src/app/actions/auth.ts @@ -15,7 +15,7 @@ import { getSession, } from '@/lib/auth'; import { logger } from '@/lib/logger'; -import { rateLimit } from '@/lib/rate-limit'; +import { rateLimitDual } from '@/lib/rate-limit'; import { hashClaimToken, PLACEHOLDER_PASSWORD_PREFIX, @@ -32,7 +32,7 @@ import { // --------------------------------------------------------------------------- export const signUpAction = createSafeAction(signUpSchema, async (data) => { - const rl = rateLimit(`signup:${data.email.toLowerCase()}`, 5, 60_000); + const rl = await rateLimitDual('signup', data.email, 20, 5, 60_000); if (!rl.allowed) { throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`); } @@ -123,9 +123,9 @@ export const signUpAction = createSafeAction(signUpSchema, async (data) => { // --------------------------------------------------------------------------- export const signInAction = createSafeAction(signInSchema, async (data) => { - // Rate-limit BEFORE any DB or scrypt work — the sync scrypt verify is - // exactly what an attacker would use to burn the event loop. - const rl = rateLimit(`signin:${data.email.toLowerCase()}`, 5, 60_000); + // Dual rate-limit BEFORE any DB or scrypt work — checking IP first protects + // both Node event loop and prevents Account Lockout DoS against target emails. + const rl = await rateLimitDual('signin', data.email, 20, 5, 60_000); if (!rl.allowed) { throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`); } diff --git a/src/lib/__tests__/rate-limit.test.ts b/src/lib/__tests__/rate-limit.test.ts new file mode 100644 index 0000000..f3cd433 --- /dev/null +++ b/src/lib/__tests__/rate-limit.test.ts @@ -0,0 +1,79 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { rateLimit, rateLimitDual, resetRateLimits } from '../rate-limit'; + +describe('rateLimit', () => { + beforeEach(() => { + resetRateLimits(); + }); + + it('allows requests within limit', () => { + const res1 = rateLimit('test-key', 2, 60_000); + expect(res1.allowed).toBe(true); + expect(res1.retryAfterSeconds).toBe(0); + + const res2 = rateLimit('test-key', 2, 60_000); + expect(res2.allowed).toBe(true); + }); + + it('blocks requests exceeding limit', () => { + rateLimit('test-key', 2, 60_000); + rateLimit('test-key', 2, 60_000); + + const res3 = rateLimit('test-key', 2, 60_000); + expect(res3.allowed).toBe(false); + expect(res3.retryAfterSeconds).toBeGreaterThan(0); + }); + + it('does not record denied hits (does not extend penalty window)', () => { + vi.useFakeTimers(); + const now = Date.now(); + vi.setSystemTime(now); + + rateLimit('test-key', 1, 60_000); // 1st hit -> allowed + rateLimit('test-key', 1, 60_000); // 2nd hit -> blocked + + // Advance time 61s + vi.setSystemTime(now + 61_000); + + const res = rateLimit('test-key', 1, 60_000); + expect(res.allowed).toBe(true); + + vi.useRealTimers(); + }); +}); + +describe('rateLimitDual', () => { + beforeEach(() => { + resetRateLimits(); + }); + + it('allows request when both IP and target limits are under thresholds', async () => { + const res = await rateLimitDual('signin', 'user@example.com', 20, 5, 60_000); + expect(res.allowed).toBe(true); + }); + + it('blocks request when target limit is exceeded', async () => { + for (let i = 0; i < 5; i++) { + await rateLimitDual('signin', 'target@example.com', 20, 5, 60_000); + } + + const res = await rateLimitDual('signin', 'target@example.com', 20, 5, 60_000); + expect(res.allowed).toBe(false); + }); + + it('blocks IP when IP limit is exceeded before checking target limit', async () => { + // Fill IP limit (3) + for (let i = 0; i < 3; i++) { + await rateLimitDual('signin', `victim${i}@example.com`, 3, 5, 60_000); + } + + // IP blocked attempt against a new target + const res = await rateLimitDual('signin', 'fresh-target@example.com', 3, 5, 60_000); + expect(res.allowed).toBe(false); + + // Ensure fresh-target bucket was NOT populated/polluted because IP limit blocked it first + resetRateLimits(); // Clear rate limits (e.g., simulating IP change or reset) + const freshRes = await rateLimitDual('signin', 'fresh-target@example.com', 3, 5, 60_000); + expect(freshRes.allowed).toBe(true); + }); +}); diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index 5d0ef23..ac5ce61 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -8,6 +8,7 @@ */ import 'server-only'; +import { headers } from 'next/headers'; const buckets = new Map(); @@ -17,6 +18,32 @@ export interface RateLimitResult { retryAfterSeconds: number; } +/** + * Reset all rate limiting buckets (useful for unit testing). + */ +export function resetRateLimits(): void { + buckets.clear(); +} + +/** + * Safely extract client IP from Next.js headers. + */ +export async function getClientIp(): Promise { + try { + const headerStore = await headers(); + const xff = headerStore.get('x-forwarded-for'); + if (xff) { + const ip = xff.split(',')[0]?.trim(); + if (ip) return ip; + } + const realIp = headerStore.get('x-real-ip'); + if (realIp) return realIp.trim(); + } catch { + // Outside request context (e.g. in tests without request context) + } + return '127.0.0.1'; +} + /** * Record a hit for `key` and report whether it stays within `limit` hits * per `windowMs`. Denied hits are not recorded (a blocked attacker doesn't @@ -47,3 +74,24 @@ export function rateLimit(key: string, limit = 5, windowMs = 60_000): RateLimitR return { allowed: true, retryAfterSeconds: 0 }; } + +/** + * Dual rate limiting: Checks IP first to block brute force / DoS before + * checking target identifier (e.g. email) bucket, preventing Account Lockout DoS. + */ +export async function rateLimitDual( + actionPrefix: string, + targetIdentifier: string, + ipLimit = 20, + targetLimit = 5, + windowMs = 60_000, +): Promise { + const ip = await getClientIp(); + // IP rate limiting executed FIRST to prevent blocked IP from polluting target bucket + const ipResult = rateLimit(`ip:${actionPrefix}:${ip}`, ipLimit, windowMs); + if (!ipResult.allowed) { + return ipResult; + } + + return rateLimit(`target:${actionPrefix}:${targetIdentifier.toLowerCase()}`, targetLimit, windowMs); +}