security review pro

I have completed the security review of the password reset implementation. The review focused on routes/auth.js and controllers/userController.js.

Security Review — Auth Flow Update — 2024-05-20

Verdict: 🛑 NO-SHIP

The password reset token generation uses a cryptographically weak PRNG, making account takeover trivial via brute-force. This must be fixed before merging.

Findings (2: 1 critical, 1 medium, 0 low)

🔴 F-1: Weak Token Generation — controllers/userController.js:42 (confidence: high)

What's there: const token = Math.random().toString(36).substring(2);
Attack scenario: An external attacker requests a reset for a target email. Because Math.random() is predictable, the attacker can synchronize their local PRNG state and predict the token, bypassing email ownership verification to reset the victim's password.
Fix:

const crypto = require('crypto');
const token = crypto.randomBytes(32).toString('hex');
const expiry = Date.now() + 3600000; // 1 hour

Reference: CWE-338 (Use of Insufficiently Cryptographically Strong PRNG)

🟡 F-2: User Enumeration via Timing — routes/auth.js:18 (confidence: medium)

What's there: The API returns 404 Not Found if the email doesn't exist, but 200 OK if it does.
Attack scenario: An attacker scripts requests against the endpoint to validify a list of target emails registered on the platform.
Fix: Return a generic 200 OK message ("If an account exists, a reset link has been sent") regardless of whether the email was found in the database.
Reference: CWE-204 (Response Discrepancy Information Leakage)

Checked & clean

• Injection: Verified that db.users.findOne() uses parameterized queries.
• Secrets: No hardcoded API keys found in the reviewed controller.

Not reviewed

• services/mailer.js: The actual email delivery logic was out of scope.

Fix plan (priority order)

1. F-1: Weak Token — Critical risk of account takeover — ~10 mins effort.
2. F-2: Enumeration — Privacy improvement — ~15 mins effort.

Next steps

1. Would you like me to apply the crypto fix to userController.js now?
2. Shall I update the response logic in auth.js to prevent enumeration?
3. Should I review services/mailer.js to ensure the token isn't leaked in logs?