From 5d3dce3a413f1b77134f0c8bc515a69cefecef4d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:49:13 +0000 Subject: [PATCH 1/3] fix(auth): implement secure dual rate-limiting Integrate rateLimitDual in signUpAction and signInAction to limit by both client IP and lowercase email address, preventing Account Lockout Denial of Service (DoS) attacks. 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 | 147 ++++++++++++++++++ src/lib/rate-limit.ts | 40 +++++ 5 files changed, 198 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..03fd132 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 - Account Lockout Denial of Service via Target-Based Bucket Pollution +**Vulnerability:** Standard rate limiting on sensitive endpoints (like sign-in or sign-up) using only target-based identifiers (such as email) allows a malicious actor to continuously hit the rate limiter with a legitimate user's email, locking that user out of their own account (Account Lockout Denial of Service). +**Learning:** Single-key or incorrect-order multi-key rate-limiting can easily be weaponized to lock out legitimate users. If the target-based check is evaluated first or if there's no IP check, an attacker can pollute the target bucket. +**Prevention:** Always implement dual rate-limiting that combines client IP-based keys (safely extracted from headers like `x-forwarded-for` and `x-real-ip` using TS-safe array access fallbacks) and target-based keys (lowercase emails). Crucially, always execute the client IP rate-limiting check before checking or incrementing target-based buckets, ensuring blocked attackers cannot lock out legitimate users. 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..597508d 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, 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, 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..1df2840 --- /dev/null +++ b/src/lib/__tests__/rate-limit.test.ts @@ -0,0 +1,147 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { rateLimit, rateLimitDual, resetRateLimits } from '../rate-limit'; +import { headers } from 'next/headers'; + +vi.mock('next/headers', () => ({ + headers: vi.fn(), +})); + +describe('rate-limit.ts', () => { + beforeEach(() => { + resetRateLimits(); + vi.useFakeTimers(); + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + describe('rateLimit', () => { + it('allows requests up to the limit and blocks exceeding ones', () => { + const key = 'test-key'; + + // Limit of 3 + expect(rateLimit(key, 3, 60_000).allowed).toBe(true); + expect(rateLimit(key, 3, 60_000).allowed).toBe(true); + expect(rateLimit(key, 3, 60_000).allowed).toBe(true); + + // 4th request within the same window should be blocked + const blocked = rateLimit(key, 3, 60_000); + expect(blocked.allowed).toBe(false); + expect(blocked.retryAfterSeconds).toBeGreaterThan(0); + }); + + it('sliding window lets requests through after the window expires', () => { + const key = 'sliding-key'; + + rateLimit(key, 2, 10_000); + vi.advanceTimersByTime(4000); + rateLimit(key, 2, 10_000); + + // 3rd request is blocked + expect(rateLimit(key, 2, 10_000).allowed).toBe(false); + + // Advance by 6001 ms (total 10,001 ms from first hit) - first hit falls out + vi.advanceTimersByTime(6001); + + // Now we should be allowed again + expect(rateLimit(key, 2, 10_000).allowed).toBe(true); + }); + + it('performs cleanup if the buckets size exceeds 10,000', () => { + // Create over 10,000 stale entries + for (let i = 0; i < 10005; i++) { + rateLimit(`key-${i}`, 1, 10_000); + } + + // Advance time so they all expire + vi.advanceTimersByTime(11_000); + + // Run another rateLimit to trigger opportunistic cleanup + rateLimit('new-key', 5, 10_000); + + // Ensure behavior remains correct for the new key + expect(rateLimit('new-key', 5, 10_000).allowed).toBe(true); + }); + }); + + describe('rateLimitDual', () => { + it('extracts IP safely and limits requests', async () => { + const mockHeaders = { + get: (h: string) => { + if (h === 'x-forwarded-for') return '203.0.113.195, 198.51.100.1'; + return null; + } + }; + (headers as unknown as ReturnType).mockResolvedValue(mockHeaders); + + // Under standard limits (target = 2, ip = 4) + const res1 = await rateLimitDual('action', 'user@example.com', 2, 60_000, 4, 60_000); + expect(res1.allowed).toBe(true); + + const res2 = await rateLimitDual('action', 'user@example.com', 2, 60_000, 4, 60_000); + expect(res2.allowed).toBe(true); + + // 3rd target attempt should block + const res3 = await rateLimitDual('action', 'user@example.com', 2, 60_000, 4, 60_000); + expect(res3.allowed).toBe(false); + }); + + it('uses x-real-ip if x-forwarded-for is missing', async () => { + const mockHeaders = { + get: (h: string) => { + if (h === 'x-real-ip') return '198.51.100.5'; + return null; + } + }; + (headers as unknown as ReturnType).mockResolvedValue(mockHeaders); + + const res = await rateLimitDual('action', 'user2@example.com', 2, 60_000, 2, 60_000); + expect(res.allowed).toBe(true); + }); + + it('falls back to unknown-ip if all headers are missing', async () => { + const mockHeaders = { + get: () => null + }; + (headers as unknown as ReturnType).mockResolvedValue(mockHeaders); + + const res = await rateLimitDual('action', 'user3@example.com', 2, 60_000, 2, 60_000); + expect(res.allowed).toBe(true); + }); + + it('blocks by IP first, preventing target lockout (bucket pollution protection)', async () => { + const mockHeaders = { + get: (h: string) => { + if (h === 'x-forwarded-for') return '1.2.3.4'; + return null; + } + }; + (headers as unknown as ReturnType).mockResolvedValue(mockHeaders); + + // Target limit: 3, IP limit: 2 + // Execute 2 requests + await rateLimitDual('login', 'legit@example.com', 3, 60_000, 2, 60_000); + await rateLimitDual('login', 'legit@example.com', 3, 60_000, 2, 60_000); + + // 3rd request should hit the IP limit first + const resBlock = await rateLimitDual('login', 'legit@example.com', 3, 60_000, 2, 60_000); + expect(resBlock.allowed).toBe(false); + + // Change the IP to simulate a different client IP trying to log in as the same legitimate user + const mockHeadersNewIP = { + get: (h: string) => { + if (h === 'x-forwarded-for') return '5.6.7.8'; + return null; + } + }; + (headers as unknown as ReturnType).mockResolvedValue(mockHeadersNewIP); + + // The new IP should be able to attempt login for 'legit@example.com' since the target limit (3) wasn't breached + // and target bucket wasn't polluted by the IP-blocked requests + const resAllowed = await rateLimitDual('login', 'legit@example.com', 3, 60_000, 2, 60_000); + expect(resAllowed.allowed).toBe(true); + }); + }); +}); diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index 5d0ef23..c063f04 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,42 @@ export function rateLimit(key: string, limit = 5, windowMs = 60_000): RateLimitR return { allowed: true, retryAfterSeconds: 0 }; } + +/** + * Dual rate limiter: rate limits based on both the client IP address (retrieved safely + * from Next.js request headers) and a target identifier (such as email). + * + * Always runs the IP check BEFORE target-based check to prevent IP-blocked malicious actors + * from lockouting legitimate accounts (Account Lockout Denial of Service). + */ +export async function rateLimitDual( + action: string, + target: string, + targetLimit = 5, + targetWindowMs = 60_000, + ipLimit = 15, + ipWindowMs = 60_000 +): Promise { + const h = await headers(); + const xff = h.get('x-forwarded-for'); + const ip = (xff ? xff.split(',')[0]?.trim() : null) || h.get('x-real-ip') || 'unknown-ip'; + + const ipKey = `${action}:ip:${ip}`; + const targetKey = `${action}:target:${target.toLowerCase()}`; + + // 1. IP check first (avoids target-based bucket pollution by blocked IPs) + const ipResult = rateLimit(ipKey, ipLimit, ipWindowMs); + if (!ipResult.allowed) { + return ipResult; + } + + // 2. Target check second + return rateLimit(targetKey, targetLimit, targetWindowMs); +} + +/** + * Resets all rate-limiting buckets (useful for clearing test pollution/interference). + */ +export function resetRateLimits(): void { + buckets.clear(); +} From f428997d52871bc2ea04b264929bd929e0c83560 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:53:33 +0000 Subject: [PATCH 2/3] fix(auth): implement secure dual rate-limiting Integrate rateLimitDual in signUpAction and signInAction to limit by both client IP and lowercase email address, preventing Account Lockout Denial of Service (DoS) attacks. Also resolved pnpm v11.13.0 broken release issue by pinning to pnpm v11.12.0. 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..3107e0b 100644 --- a/package.json +++ b/package.json @@ -72,5 +72,5 @@ "eslint --fix" ] }, - "packageManager": "pnpm@11.13.0+sha512.88d94724d8f2e6c186744a5584c6e59ecac869ec7ba15e9cb4cd628e8dc7066820b2481d8ee3b51ea8da323a7378068aa58c556a3720d32b7c20a051d088363a" + "packageManager": "pnpm@11.12.0" } From d3e7c1d98efd82de2ac6eb8101b3ca5d70ebd6a1 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:58:07 +0000 Subject: [PATCH 3/3] fix(auth): implement secure dual rate-limiting Integrate rateLimitDual in signUpAction and signInAction to limit by both client IP and lowercase email address, preventing Account Lockout Denial of Service (DoS) attacks. Also resolved pnpm v11.13.0 broken release issue by pinning to pnpm v9.15.2 and configuring workspace packages. Co-authored-by: projectamazonph <286085559+projectamazonph@users.noreply.github.com> --- package.json | 2 +- pnpm-workspace.yaml | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 3107e0b..dc510cb 100644 --- a/package.json +++ b/package.json @@ -72,5 +72,5 @@ "eslint --fix" ] }, - "packageManager": "pnpm@11.12.0" + "packageManager": "pnpm@9.15.2" } diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index c01f7c7..5b748ec 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -1,3 +1,6 @@ +packages: + - "." + onlyBuiltDependencies: - "@prisma/client" - "@prisma/engines"