Conversation
Automated security fix generated by OrbisAI Security
|
Automated review has started. I am checking this pull request now. ⚙️ Runtime environment
🤖 Created By GHBot |
lezi-fun
left a comment
lezi-fun
left a comment
There was a problem hiding this comment.
Request changes
Blocking — the account key is unavailable at this middleware position
authLimiter is mounted before express.json() and express.urlencoded(), so req.body is not parsed when keyGenerator runs. For JSON login and registration requests, account is therefore empty and the new account-aware behavior is not applied.
Blocking — the combined key can bypass the IP limit
Even if body parsing is moved earlier, using IP plus a user-controlled email or username as one key lets an attacker rotate the account value and obtain a fresh bucket for every request. That weakens the previous per-IP protection instead of adding an independent account limit.
Please use separate limiters or a composite policy with independent per-IP and per-account counters. Add tests covering repeated attempts for one account, repeated attempts across many account values from one IP, successful requests, and requests with malformed or missing bodies.
…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 <noreply@anthropic.com>
|
Automated review could not complete for commit ⚙️ Runtime environment
🤖 Created By GHBot |
…-307-auth-rate-limit # Conflicts: # server.js
|
Automated review could not complete for commit ⚙️ Runtime environment
🤖 Created By GHBot |
Addressed. Pls review. |
2 participants
Rate limiting on authentication endpoints allows 20 attempts per 15 minutes per IP address, which is insufficient to prevent modern credential stuffing attacks. The IP-based limiting can be bypassed using proxy rotation or distributed attack infrastructure. No progressive delays, CAPTCHA challenges, or account-level rate limiting are implemented. This is defence-in-depth at
server.js:55rather than a vulnerability I can show is exploitable here — it makes the failure mode explicit and bounded. Close it freely if the pattern is intentional.Reference: CWE-307
What changed
server.jsVerification
No automated check could be run against this repository, so this change is unverified beyond review. Please treat it as a suggestion.
Automated security fix by OrbisAI Security