From a43145f4268d3633c35dfb6a75f2928831d5e941 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:50:34 +0000 Subject: [PATCH 1/2] sec(auth): implement asynchronous dual rate limiting to prevent account lockout DoS Co-authored-by: projectamazonph <286085559+projectamazonph@users.noreply.github.com> --- .jules/sentinel.md | 5 + .../actions/__tests__/auth-actions.test.ts | 3 + src/app/actions/auth.ts | 6 +- src/lib/__tests__/rate-limit.test.ts | 121 ++++++++++++++++++ src/lib/rate-limit.ts | 41 ++++++ 5 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 src/lib/__tests__/rate-limit.test.ts diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 29abdfb..db1d651 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -1,5 +1,10 @@ # Sentinel Journal — Critical Security Learnings +## 2026-07-17 - Dual Rate Limiting Prevents Account Lockout DoS +**Vulnerability:** Simple rate limiting on sensitive authentication actions (sign-in/sign-up) based purely on target-specific identifiers (e.g., username/email) enables malicious actors to trigger rate limits for legitimate users. An attacker could flood authentication requests for a specific user email, resulting in an Account Lockout Denial of Service (DoS) for the victim. +**Learning:** Rate limiting must protect both system resources and user experience. Enforcing target-based rate limits before client IP rate limits allows malicious IPs to pollute and exhaust the target's limit bucket. +**Prevention:** Implement a dual rate-limiting utility where client IP-based limits are verified and recorded *before* target-based limits. This ensures that malicious IP addresses are blocked first, preventing them from exhausting the target's quota and preserving account availability for the legitimate owner. + ## 2026-07-16 - Synchronous Password Hashing Blocks Next.js Event Loop (DoS Risk) **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. diff --git a/src/app/actions/__tests__/auth-actions.test.ts b/src/app/actions/__tests__/auth-actions.test.ts index 766ee9b..e2632bb 100644 --- a/src/app/actions/__tests__/auth-actions.test.ts +++ b/src/app/actions/__tests__/auth-actions.test.ts @@ -29,6 +29,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..17d2790 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, 10, 5, 60_000); if (!rl.allowed) { throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`); } @@ -125,7 +125,7 @@ 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); + const rl = await rateLimitDual('signin', data.email, 10, 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..bc9b80a --- /dev/null +++ b/src/lib/__tests__/rate-limit.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { rateLimit, rateLimitDual, resetRateLimits } from '../rate-limit'; + +// Mock 'next/headers' as requested +const mockHeadersGet = vi.fn(); +vi.mock('next/headers', () => ({ + headers: () => Promise.resolve({ + get: mockHeadersGet, + }), +})); + +describe('rate-limit.ts', () => { + beforeEach(() => { + resetRateLimits(); + vi.useFakeTimers(); + mockHeadersGet.mockReset(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + describe('rateLimit', () => { + it('allows hits within the limit and blocks exceeding hits', () => { + // Limit of 3 hits in 60s + expect(rateLimit('key1', 3, 60_000).allowed).toBe(true); + expect(rateLimit('key1', 3, 60_000).allowed).toBe(true); + expect(rateLimit('key1', 3, 60_000).allowed).toBe(true); + + const blocked = rateLimit('key1', 3, 60_000); + expect(blocked.allowed).toBe(false); + expect(blocked.retryAfterSeconds).toBe(60); + }); + + it('advances the window correctly over time', () => { + expect(rateLimit('key1', 2, 10_000).allowed).toBe(true); + vi.advanceTimersByTime(6000); + expect(rateLimit('key1', 2, 10_000).allowed).toBe(true); + + // Exceeded + expect(rateLimit('key1', 2, 10_000).allowed).toBe(false); + + // Advance so first hit falls out of the window + vi.advanceTimersByTime(5000); // Total 11s elapsed since 1st hit + expect(rateLimit('key1', 2, 10_000).allowed).toBe(true); + }); + + it('cleans up buckets opportunistically when map is too large', () => { + // Trigger opportunistic cleanup (buckets.size > 10_000) + for (let i = 0; i < 10005; i++) { + rateLimit(`key-${i}`, 1, 10); + } + // Advance time so everything is cutoff + vi.advanceTimersByTime(20); + // This call triggers cleanup + rateLimit('newkey', 1, 10); + // The old keys should have been cleaned up and removed. + }); + }); + + describe('rateLimitDual', () => { + it('uses x-forwarded-for header for client IP rate limiting', async () => { + mockHeadersGet.mockImplementation((header: string) => { + if (header === 'x-forwarded-for') return '1.2.3.4, 5.6.7.8'; + return null; + }); + + // Under IP limit of 2, target limit of 1 + // First attempt: should succeed for both IP and target + const res1 = await rateLimitDual('login', 'user@example.com', 2, 1, 60_000); + expect(res1.allowed).toBe(true); + + // Second attempt with same email: target limit exceeded (limit is 1) + const res2 = await rateLimitDual('login', 'user@example.com', 2, 1, 60_000); + expect(res2.allowed).toBe(false); + }); + + it('IP limit check runs BEFORE target check to prevent account lockout', async () => { + mockHeadersGet.mockImplementation((header: string) => { + if (header === 'x-forwarded-for') return '9.9.9.9'; + return null; + }); + + // IP limit = 1, target limit = 5 + // First attempt: allowed + const res1 = await rateLimitDual('login', 'legit@example.com', 1, 5, 60_000); + expect(res1.allowed).toBe(true); + + // Second attempt: IP blocked (since IP limit is 1) + const res2 = await rateLimitDual('login', 'legit@example.com', 1, 5, 60_000); + expect(res2.allowed).toBe(false); + + // Verify target-based key did not get polluted by checking another IP + mockHeadersGet.mockImplementation((header: string) => { + if (header === 'x-forwarded-for') return '8.8.8.8'; // safe IP + return null; + }); + + // Legit user on safe IP can still log in because their target bucket wasn't polluted by blocked IP + const res3 = await rateLimitDual('login', 'legit@example.com', 1, 5, 60_000); + expect(res3.allowed).toBe(true); + }); + + it('falls back to x-real-ip when x-forwarded-for is missing', async () => { + mockHeadersGet.mockImplementation((header: string) => { + if (header === 'x-real-ip') return '127.0.0.1'; + return null; + }); + + const res = await rateLimitDual('login', 'user@example.com', 1, 1, 60_000); + expect(res.allowed).toBe(true); + }); + + it('falls back to unknown when both headers are missing', async () => { + mockHeadersGet.mockReturnValue(null); + + const res = await rateLimitDual('login', 'user@example.com', 1, 1, 60_000); + expect(res.allowed).toBe(true); + }); + }); +}); diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index 5d0ef23..0529917 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(); @@ -47,3 +48,43 @@ export function rateLimit(key: string, limit = 5, windowMs = 60_000): RateLimitR return { allowed: true, retryAfterSeconds: 0 }; } + +/** + * Perform dual rate limiting to prevent both brute-force attacks and target-based lockout DoS. + * + * Always runs the client IP check BEFORE checking target-based keys (such as email) so that + * blocked IPs cannot fill/exhaust a legitimate user's target rate-limiting bucket. + */ +export async function rateLimitDual( + action: string, + targetKey: string, + ipLimit = 10, + targetLimit = 5, + windowMs = 60_000, +): Promise { + const reqHeaders = await headers(); + const xff = reqHeaders.get('x-forwarded-for'); + const xri = reqHeaders.get('x-real-ip'); + + // Safely extract client IP from x-forwarded-for (handling noUncheckedIndexedAccess) + const clientIp = (xff ? xff.split(',')[0]?.trim() : null) || xri || 'unknown'; + + // 1. Check IP-based rate limiting first + const ipKey = `ip:${action}:${clientIp}`; + const ipResult = rateLimit(ipKey, ipLimit, windowMs); + if (!ipResult.allowed) { + return ipResult; + } + + // 2. Check target-based rate limiting second (using lowercase keys for case insensitivity) + const targetKeyLower = targetKey.toLowerCase(); + const targetKeyFormatted = `target:${action}:${targetKeyLower}`; + return rateLimit(targetKeyFormatted, targetLimit, windowMs); +} + +/** + * Resets the in-memory rate-limiting maps to prevent test pollution. + */ +export function resetRateLimits(): void { + buckets.clear(); +} From 017dc36d5be678a94821f4b47d89620e97d1b302 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:56:02 +0000 Subject: [PATCH 2/2] sec(auth): implement asynchronous dual rate limiting to prevent account lockout DoS Co-authored-by: projectamazonph <286085559+projectamazonph@users.noreply.github.com> --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e60c440..3455125 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+sha512.ZsGsTH1HYtbX3eRMfz5ac1ke0KCAbnUdTtMtTwBPJbIoWpBrH9ip4+Yh3ztOKFi/iOUODPYmvtvpd/5DSlyvhQ==" }