From f147f4b248195a7ca328785369bbac246aed438f Mon Sep 17 00:00:00 2001 From: anupamme Date: Mon, 21 Sep 2026 13:37:33 +0000 Subject: [PATCH 1/2] fix: multi_agent.cwe-307 security vulnerability Automated security fix generated by OrbisAI Security --- server.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/server.js b/server.js index 562ab45..05f21fb 100644 --- a/server.js +++ b/server.js @@ -6,7 +6,7 @@ const express = require('express'); const cors = require('cors'); const helmet = require('helmet'); -const { rateLimit } = require('express-rate-limit'); +const { rateLimit, ipKeyGenerator } = require('express-rate-limit'); const bcrypt = require('bcryptjs'); const jwt = require('jsonwebtoken'); const { Pool } = require('pg'); @@ -85,9 +85,14 @@ const apiLimiter = rateLimit({ }); const authLimiter = rateLimit({ windowMs: 15 * 60 * 1000, - limit: 20, + limit: 5, standardHeaders: 'draft-8', legacyHeaders: false, + skipSuccessfulRequests: true, + keyGenerator: (req, res) => { + const account = String((req.body && (req.body.email || req.body.username)) || '').trim().toLowerCase(); + return account ? `${ipKeyGenerator(req, res)}:${account}` : ipKeyGenerator(req, res); + }, handler: (req, res) => res.status(429).json({ error: '尝试次数过多，请稍后重试' }), }); From 381b98bf1f45cf7266a34a34ddfdd5529e1f3174 Mon Sep 17 00:00:00 2001 From: OrbisAI Security Date: Tue, 22 Sep 2026 18:27:54 +0530 Subject: [PATCH 2/2] fix: split auth rate limiter into independent per-IP and per-account buckets authLimiter was mounted before express.json()/urlencoded(), so its keyGenerator read req.body while it was still unparsed, and combining IP with a client-controlled account value into one key let an attacker rotate the account to get a fresh bucket per request, bypassing the per-IP limit. Replace it with authIpLimiter (unchanged position, IP-only key) and authAccountLimiter (mounted after body parsing, account-only key), so neither counter can be bypassed by manipulating the other's input. Co-Authored-By: Claude Sonnet 5 --- server.js | 34 +++++++++++--- test/rate-limit.test.js | 99 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 7 deletions(-) create mode 100644 test/rate-limit.test.js diff --git a/server.js b/server.js index 05f21fb..e4c9ec1 100644 --- a/server.js +++ b/server.js @@ -6,7 +6,7 @@ const express = require('express'); const cors = require('cors'); const helmet = require('helmet'); -const { rateLimit, ipKeyGenerator } = require('express-rate-limit'); +const { rateLimit } = require('express-rate-limit'); const bcrypt = require('bcryptjs'); const jwt = require('jsonwebtoken'); const { Pool } = require('pg'); @@ -83,23 +83,43 @@ const apiLimiter = rateLimit({ legacyHeaders: false, handler: (req, res) => res.status(429).json({ error: '请求过于频繁，请稍后重试' }), }); -const authLimiter = rateLimit({ +// Per-IP bucket — default keyGenerator (IP only). Independent of any value +// the client puts in the body, so it can't be bypassed by rotating account. +const authIpLimiter = rateLimit({ windowMs: 15 * 60 * 1000, limit: 5, standardHeaders: 'draft-8', legacyHeaders: false, skipSuccessfulRequests: true, - keyGenerator: (req, res) => { - const account = String((req.body && (req.body.email || req.body.username)) || '').trim().toLowerCase(); - return account ? `${ipKeyGenerator(req, res)}:${account}` : ipKeyGenerator(req, res); - }, handler: (req, res) => res.status(429).json({ error: '尝试次数过多，请稍后重试' }), }); +function authAccountKey(req) { + return String((req.body && (req.body.email || req.body.username)) || '').trim().toLowerCase(); +} + +// Per-account bucket — independent of source IP, so spreading attempts +// across many IPs against one account is still capped. Needs req.body, so +// it must be mounted after the body parsers. Requests with no usable +// account value skip this limiter; authIpLimiter above still applies to them. +const authAccountLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + limit: 5, + standardHeaders: 'draft-8', + legacyHeaders: false, + skipSuccessfulRequests: true, + skip: (req) => !authAccountKey(req), + keyGenerator: authAccountKey, + handler: (req, res) => res.status(429).json({ error: '尝试次数过多，请稍后重试' }), +}); + +const AUTH_PATHS = ['/api/auth/login', '/api/auth/register', '/api/auth/reset-password']; + app.use('/api', apiLimiter); -app.use(['/api/auth/login', '/api/auth/register', '/api/auth/reset-password'], authLimiter); +app.use(AUTH_PATHS, authIpLimiter); app.use(express.json({ limit: '100kb' })); app.use(express.urlencoded({ extended: true, limit: '100kb' })); +app.use(AUTH_PATHS, authAccountLimiter); const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; diff --git a/test/rate-limit.test.js b/test/rate-limit.test.js new file mode 100644 index 0000000..f4e056d --- /dev/null +++ b/test/rate-limit.test.js @@ -0,0 +1,99 @@ +process.env.TRUST_PROXY = 'true'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const request = require('supertest'); +const bcrypt = require('bcryptjs'); +const { app, pool } = require('../server'); + +test.after(async () => { + await pool.end(); +}); + +let ipCounter = 0; +function nextIp() { + ipCounter += 1; + return `10.0.0.${ipCounter}`; +} + +test('repeated failed attempts for one account trip the account limiter even from different IPs', async () => { + const email = 'account-limit-test@example.test'; + for (let i = 0; i < 5; i += 1) { + const res = await request(app) + .post('/api/auth/login') + .set('X-Forwarded-For', nextIp()) + .send({ email, password: 'wrong-password' }); + assert.notEqual(res.status, 429); + } + const blocked = await request(app) + .post('/api/auth/login') + .set('X-Forwarded-For', nextIp()) + .send({ email, password: 'wrong-password' }); + assert.equal(blocked.status, 429); + assert.equal(blocked.body.error, '尝试次数过多，请稍后重试'); +}); + +test('repeated failed attempts across many accounts from one IP trip the IP limiter', async () => { + const ip = nextIp(); + for (let i = 0; i < 5; i += 1) { + const res = await request(app) + .post('/api/auth/login') + .set('X-Forwarded-For', ip) + .send({ email: `rotating-${i}@example.test`, password: 'wrong-password' }); + assert.notEqual(res.status, 429); + } + const blocked = await request(app) + .post('/api/auth/login') + .set('X-Forwarded-For', ip) + .send({ email: 'rotating-final@example.test', password: 'wrong-password' }); + assert.equal(blocked.status, 429); + assert.equal(blocked.body.error, '尝试次数过多，请稍后重试'); +}); + +test('successful logins are not counted against the auth limiters', async (t) => { + const email = 'success-test@example.test'; + const password = 'correct horse battery staple'; + const hash = await bcrypt.hash(password, 4); + + t.mock.method(pool, 'query', async () => ({ + rows: [{ + id: 1, + email, + password_hash: hash, + username: 'tester', + avatar_url: '', + bio: '', + role: 'user', + signature: '', + }], + })); + + const ip = nextIp(); + for (let i = 0; i < 8; i += 1) { + const res = await request(app) + .post('/api/auth/login') + .set('X-Forwarded-For', ip) + .send({ email, password }); + assert.equal(res.status, 200); + } +}); + +test('missing body fields fall back to the IP-only bucket without crashing', async () => { + const ip = nextIp(); + const res = await request(app) + .post('/api/auth/login') + .set('X-Forwarded-For', ip) + .send({}); + assert.equal(res.status, 400); +}); + +test('malformed JSON bodies are rejected without crashing the server', async () => { + const ip = nextIp(); + const res = await request(app) + .post('/api/auth/login') + .set('X-Forwarded-For', ip) + .set('Content-Type', 'application/json') + .send('{not valid json'); + assert.ok(res.status >= 400); + assert.ok(res.body.error); +});