diff --git a/backend/.env.example b/backend/.env.example index c4fd6a89..123bf3da 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -4,6 +4,12 @@ # Server Configuration PORT=8000 +# Comma-separated CIDRs of trusted reverse proxies (e.g. your hosting +# platform's egress ranges). X-Forwarded-For is ONLY honored from these +# addresses, so rate limiting cannot be bypassed by spoofing the header. +# Leave unset when the server is directly reachable. +# REVERSE_PROXY_CIDR=10.0.0.0/8 + # Base URL for constructing absolute URLs (e.g. for image uploads) # In production, set this to your actual domain like https://preppilot-backend.onrender.com # BASE_URL=https://preppilot-backend.onrender.com diff --git a/backend/middlewares/rateLimiter.js b/backend/middlewares/rateLimiter.js index f3ff22aa..cbf952c9 100644 --- a/backend/middlewares/rateLimiter.js +++ b/backend/middlewares/rateLimiter.js @@ -1,9 +1,18 @@ const rateLimit = require('express-rate-limit'); +// Rate-limit key derived from the trusted client IP. req.ip honors +// X-Forwarded-For only from the configured trusted proxy CIDR(s); without one +// it is the direct socket address, so a spoofed header cannot rotate the limit +// bucket. The library's ipKeyGenerator helper masks IPv6 addresses to a subnet +// so IPv6 callers cannot dodge limits by rotating addresses. +const ipKeyGenerator = (req) => + rateLimit.ipKeyGenerator(req.ip || req.socket?.remoteAddress || "unknown"); + // Login endpoint: strict brute-force protection (10 attempts per 15 minutes) const loginLimiter = rateLimit({ windowMs: 15 * 60 * 1000, // 15 minutes max: 10, // Limit each IP to 10 login attempts per window + keyGenerator: ipKeyGenerator, message: { error: 'Too many login attempts. Your account is temporarily locked. Please try again after 15 minutes.' }, standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers legacyHeaders: false, // Disable the `X-RateLimit-*` headers @@ -14,6 +23,7 @@ const loginLimiter = rateLimit({ const authLimiter = rateLimit({ windowMs: 15 * 60 * 1000, // 15 minutes max: 50, // Limit each IP to 50 requests per `window` + keyGenerator: ipKeyGenerator, message: { error: 'Too many registration or authentication attempts, please try again after 15 minutes.' }, standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers legacyHeaders: false, // Disable the `X-RateLimit-*` headers @@ -24,6 +34,7 @@ const authLimiter = rateLimit({ const aiLimiter = rateLimit({ windowMs: 60 * 60 * 1000, // 1 hour max: 20, // Limit each IP to 20 requests per `window` (here, per hour) + keyGenerator: ipKeyGenerator, message: { error: 'AI generation limit reached (20 requests per hour) to prevent API abuse. Please try again later.' }, standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers legacyHeaders: false, // Disable the `X-RateLimit-*` headers @@ -33,6 +44,7 @@ const aiLimiter = rateLimit({ const generalLimiter = rateLimit({ windowMs: 15 * 60 * 1000, // 15 minutes max: 100, // Limit each IP to 100 requests per `window` (here, per 15 minutes) + keyGenerator: ipKeyGenerator, message: { error: 'Too many requests, please try again after 15 minutes.' }, standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers legacyHeaders: false, // Disable the `X-RateLimit-*` headers @@ -42,6 +54,7 @@ const generalLimiter = rateLimit({ const sensitiveAuthLimiter = rateLimit({ windowMs: 15 * 60 * 1000, // 15 minutes max: 10, // Limit each IP to 10 sensitive account requests per window + keyGenerator: ipKeyGenerator, message: { error: 'Too many sensitive account action attempts, please try again after 15 minutes.' }, standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers legacyHeaders: false, // Disable the `X-RateLimit-*` headers diff --git a/backend/server.js b/backend/server.js index 1a6341b8..e321c0db 100644 --- a/backend/server.js +++ b/backend/server.js @@ -29,7 +29,18 @@ const { generalHeaders, sensitiveRouteHeaders } = require("./middlewares/securit const { uploadsStaticHeaders } = require("./middlewares/uploadMiddleware"); const app = express(); -app.set("trust proxy", 1); +// Trust X-Forwarded-For only when it comes from a known reverse proxy. The +// previous blanket `trust proxy: 1` let any client spoof the header and rotate +// their IP on every request, defeating every rate limiter. With no CIDR +// configured the header is ignored entirely and req.ip is the direct socket +// address, so it cannot be reset by the caller. +const reverseProxyCidrs = (process.env.REVERSE_PROXY_CIDR || "") + .split(",") + .map((cidr) => cidr.trim()) + .filter(Boolean); +if (reverseProxyCidrs.length > 0) { + app.set("trust proxy", reverseProxyCidrs); +} app.use(generalHeaders); const isDev = process.env.NODE_ENV !== "production"; const originEnvList = [ diff --git a/backend/tests/rateLimiter.xff.bypass.unit.test.js b/backend/tests/rateLimiter.xff.bypass.unit.test.js new file mode 100644 index 00000000..43c7f132 --- /dev/null +++ b/backend/tests/rateLimiter.xff.bypass.unit.test.js @@ -0,0 +1,40 @@ +import { describe, it, expect, beforeAll } from "vitest"; +import express from "express"; +import request from "supertest"; +import { loginLimiter } from "../middlewares/rateLimiter.js"; + +// --------------------------------------------------------------------------- +// Rate-limit bypass fix (issue #1438): with no trusted reverse proxy +// configured, a client-supplied X-Forwarded-For header must NOT rotate the +// rate-limit bucket. req.ip stays the direct socket address, so the 11th login +// attempt is still rejected even while spoofing a different IP each request. +// --------------------------------------------------------------------------- + +let app; + +beforeAll(() => { + app = express(); + app.use(express.json()); + app.post("/login", loginLimiter, (req, res) => res.status(200).json({ ok: true })); +}); + +describe("login limiter keyed on the trusted client IP", () => { + it("rejects the 11th attempt even when X-Forwarded-For is rotated per request", async () => { + const statuses = []; + for (let i = 0; i <= 10; i++) { + const res = await request(app) + .post("/login") + .set("X-Forwarded-For", `203.0.113.${i}`) + .send({ email: `user${i}@example.com`, password: "wrong" }); + statuses.push(res.status); + } + + // The first 10 requests pass through to the handler (200), proving the + // spoofed header did not create a fresh bucket for each attempt. + for (let i = 0; i < 10; i++) { + expect(statuses[i]).toBe(200); + } + // The 11th attempt is rejected by loginLimiter (max: 10). + expect(statuses[10]).toBe(429); + }); +});