diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 29abdfb..b8f0b3b 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-20 - Dual Rate-Limiting Protects Against Multi-Vector Brute-Force Attacks +**Vulnerability:** Authentication endpoints (`signInAction` and `signUpAction`) only rate-limited on a single target key (`signin:${email}`). This left the system vulnerable to distributed dictionary/credential stuffing attacks (same target from multiple IPs) and multi-target credential stuffing (multiple emails targeted from the same IP, bypassing target-based rate limits). +**Learning:** Single-key rate limiting is insufficient for modern high-value web applications. Attackers can distribute requests across thousands of accounts from a single IP to bypass target-locked throttling. +**Prevention:** Always implement dual rate-limiting combining both IP-based keys (retrieved safely via headers like `x-forwarded-for`/`x-real-ip`) and target-based keys (such as lowercase email addresses). diff --git a/src/__tests__/setup.ts b/src/__tests__/setup.ts index 50336ab..138985f 100644 --- a/src/__tests__/setup.ts +++ b/src/__tests__/setup.ts @@ -8,6 +8,9 @@ vi.mock('next/headers', () => ({ set: vi.fn(), delete: vi.fn(), }), + headers: () => Promise.resolve({ + get: () => null, + }), })); vi.mock('next/navigation', () => ({ diff --git a/src/app/actions/__tests__/auth-actions.test.ts b/src/app/actions/__tests__/auth-actions.test.ts index 766ee9b..69d563a 100644 --- a/src/app/actions/__tests__/auth-actions.test.ts +++ b/src/app/actions/__tests__/auth-actions.test.ts @@ -23,12 +23,19 @@ vi.mock('@/lib/db', () => ({ }, })); +const { mockHeaders } = vi.hoisted(() => ({ + mockHeaders: { + get: vi.fn().mockReturnValue(null), + }, +})); + vi.mock('next/headers', () => ({ cookies: () => ({ get: () => undefined, set: vi.fn(), delete: vi.fn(), }), + headers: () => Promise.resolve(mockHeaders), })); vi.mock('next/navigation', () => ({ @@ -139,4 +146,60 @@ describe('auth actions', () => { expect(result.success).toBe(true); expect((result as any).data.ok).toBe(true); }); + + describe('dual rate-limiting', () => { + beforeEach(() => { + mockHeaders.get.mockImplementation((header: string) => { + if (header === 'x-forwarded-for') return '203.0.113.195'; + return null; + }); + }); + + it('rate-limits by email on multiple sign-in attempts for same email', async () => { + (db.user.findUnique as unknown as ReturnType).mockResolvedValue(null); + + // Trigger rate limit with 5 allowed requests + for (let i = 0; i < 5; i++) { + const res = await signInAction({ email: 'target@example.com', password: 'x' }); + expect(res.success).toBe(false); + if (!res.success) { + expect(res.error).toBe('Email or password is incorrect.'); + } + } + + // 6th request triggers email rate limit + const blockedRes = await signInAction({ email: 'target@example.com', password: 'x' }); + expect(blockedRes.success).toBe(false); + if (!blockedRes.success) { + expect(blockedRes.error).toMatch(/Too many attempts\. Try again in \d+s\./); + } + }); + + it('rate-limits by IP on multiple sign-in attempts from same IP but different emails', async () => { + (db.user.findUnique as unknown as ReturnType).mockResolvedValue(null); + + // Use a distinct IP + mockHeaders.get.mockImplementation((header: string) => { + if (header === 'x-forwarded-for') return '198.51.100.42'; + return null; + }); + + // 10 attempts on different emails from same IP are allowed to check credentials (but fail credential check) + for (let i = 0; i < 10; i++) { + const email = `user-${i}@example.com`; + const res = await signInAction({ email, password: 'x' }); + expect(res.success).toBe(false); + if (!res.success) { + expect(res.error).toBe('Email or password is incorrect.'); + } + } + + // 11th request triggers IP rate limit + const blockedRes = await signInAction({ email: 'user-11@example.com', password: 'x' }); + expect(blockedRes.success).toBe(false); + if (!blockedRes.success) { + expect(blockedRes.error).toBe('Too many attempts from this IP. Try again in 60s.'); + } + }); + }); }); diff --git a/src/app/actions/__tests__/tool-actions.test.ts b/src/app/actions/__tests__/tool-actions.test.ts index 5a60674..da51a2d 100644 --- a/src/app/actions/__tests__/tool-actions.test.ts +++ b/src/app/actions/__tests__/tool-actions.test.ts @@ -35,6 +35,9 @@ vi.mock('next/headers', () => ({ set: vi.fn(), delete: vi.fn(), }), + headers: () => Promise.resolve({ + get: () => null, + }), })) vi.mock('next/navigation', () => ({ diff --git a/src/app/actions/auth.ts b/src/app/actions/auth.ts index 2f10075..c31e019 100644 --- a/src/app/actions/auth.ts +++ b/src/app/actions/auth.ts @@ -5,6 +5,7 @@ 'use server'; import { redirect } from 'next/navigation'; +import { headers } from 'next/headers'; import { db } from '@/lib/db'; import { hashPassword, @@ -32,9 +33,18 @@ import { // --------------------------------------------------------------------------- export const signUpAction = createSafeAction(signUpSchema, async (data) => { - const rl = rateLimit(`signup:${data.email.toLowerCase()}`, 5, 60_000); - if (!rl.allowed) { - throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`); + const heads = await headers(); + const xff = heads.get('x-forwarded-for'); + const ip = (xff ? xff.split(',')[0]?.trim() : null) ?? heads.get('x-real-ip') ?? 'unknown'; + + const rlEmail = rateLimit(`signup:email:${data.email.toLowerCase()}`, 5, 60_000); + if (!rlEmail.allowed) { + throw new Error(`Too many attempts. Try again in ${rlEmail.retryAfterSeconds}s.`); + } + + const rlIp = rateLimit(`signup:ip:${ip}`, 10, 60_000); + if (!rlIp.allowed) { + throw new Error(`Too many attempts from this IP. Try again in ${rlIp.retryAfterSeconds}s.`); } const existing = await db.user.findUnique({ where: { email: data.email } }); @@ -123,11 +133,20 @@ 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); - if (!rl.allowed) { - throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`); + const heads = await headers(); + const xff = heads.get('x-forwarded-for'); + const ip = (xff ? xff.split(',')[0]?.trim() : null) ?? heads.get('x-real-ip') ?? 'unknown'; + + // Rate-limit BEFORE any DB or scrypt work — the async scrypt verify is + // protected from event-loop starvation by dual rate-limiting (IP & target). + const rlEmail = rateLimit(`signin:email:${data.email.toLowerCase()}`, 5, 60_000); + if (!rlEmail.allowed) { + throw new Error(`Too many attempts. Try again in ${rlEmail.retryAfterSeconds}s.`); + } + + const rlIp = rateLimit(`signin:ip:${ip}`, 10, 60_000); + if (!rlIp.allowed) { + throw new Error(`Too many attempts from this IP. Try again in ${rlIp.retryAfterSeconds}s.`); } const user = await db.user.findUnique({ where: { email: data.email } }); diff --git a/src/lib/__tests__/rate-limit.test.ts b/src/lib/__tests__/rate-limit.test.ts new file mode 100644 index 0000000..9809eb7 --- /dev/null +++ b/src/lib/__tests__/rate-limit.test.ts @@ -0,0 +1,98 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { rateLimit } from '../rate-limit'; + +describe('rate-limit.ts', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('allows requests within limit and then blocks', () => { + const key = 'user_1'; + + // Default limit is 5 per 60,000 ms + for (let i = 0; i < 5; i++) { + const result = rateLimit(key); + expect(result.allowed).toBe(true); + expect(result.retryAfterSeconds).toBe(0); + } + + // 6th request is blocked + const blockedResult = rateLimit(key); + expect(blockedResult.allowed).toBe(false); + expect(blockedResult.retryAfterSeconds).toBe(60); // 60 seconds remaining + }); + + it('respects custom limits and windows', () => { + const key = 'custom_user'; + const limit = 3; + const windowMs = 10_000; + + for (let i = 0; i < limit; i++) { + const result = rateLimit(key, limit, windowMs); + expect(result.allowed).toBe(true); + } + + const blockedResult = rateLimit(key, limit, windowMs); + expect(blockedResult.allowed).toBe(false); + expect(blockedResult.retryAfterSeconds).toBe(10); + }); + + it('allows request again after window expires', () => { + const key = 'expiring_user'; + const limit = 2; + const windowMs = 10_000; + + expect(rateLimit(key, limit, windowMs).allowed).toBe(true); + expect(rateLimit(key, limit, windowMs).allowed).toBe(true); + expect(rateLimit(key, limit, windowMs).allowed).toBe(false); + + // Fast-forward time by 11 seconds + vi.advanceTimersByTime(11_000); + + // Should be allowed again + const result = rateLimit(key, limit, windowMs); + expect(result.allowed).toBe(true); + }); + + it('does not record blocked attempts to extend lockout', () => { + const key = 'blocked_lockout_user'; + const limit = 2; + const windowMs = 10_000; + + expect(rateLimit(key, limit, windowMs).allowed).toBe(true); + expect(rateLimit(key, limit, windowMs).allowed).toBe(true); + + // First blocked request at T=0 + expect(rateLimit(key, limit, windowMs).allowed).toBe(false); + + // Advance time by 6 seconds (oldest hit is still active) + vi.advanceTimersByTime(6000); + + // Try again - still blocked + expect(rateLimit(key, limit, windowMs).allowed).toBe(false); + + // Advance by another 5 seconds (T=11 seconds total) + vi.advanceTimersByTime(5000); + + // Should be allowed now, because blocked requests didn't record new hits + expect(rateLimit(key, limit, windowMs).allowed).toBe(true); + }); + + it('performs opportunistic cleanup of old buckets when map grows large', () => { + // Fill buckets up to 10,001 unique keys + for (let i = 0; i < 10005; i++) { + rateLimit(`key_${i}`, 1, 10_000); + } + + // Now advance time so all those buckets are expired + vi.advanceTimersByTime(15_000); + + // Hit rateLimit again to trigger cleanup block + const result = rateLimit('new_key_after_cleanup', 1, 10_000); + expect(result.allowed).toBe(true); + }); +});