[Security]: OTPs and password-reset codes are stored in plain text, and /verify-otp discloses which emails have accounts

[MOHITKOURAV01]

Summary

Every pending verification code and password-reset code in this application is stored in the database as plain text, and the endpoints that consume them tell an unauthenticated caller which email addresses have accounts.

registerController and forgotPasswordController both do this:

const otp = Math.floor(100000 + Math.random() * 900000).toString();
const otpExpiry = new Date(Date.now() + 10 * 60 * 1000);
// ...
const newUser = new userSchema({ ..., otp, otpExpiry });

const resetToken = Math.floor(100000 + Math.random() * 900000).toString();
user.resetToken = resetToken;
user.resetTokenExpiry = resetTokenExpiry;

userModel marks both select: false, which stops a stray find() from returning them, and #54 removed them from the admin projection for the same reason. Neither measure changes what is on disk. A database backup, a replica with read-only credentials, a mongodump in a CI artefact or anything else with read access to the users collection carries a live credential for every account with a pending reset — the holder can complete /api/user/reset-password for each of them without ever seeing the mailbox.

The comparisons are plain !== on that secret material:

if (user.otp !== otp || user.otpExpiry < Date.now()) { ... }
if (user.resetToken !== token || user.resetTokenExpiry < Date.now()) { ... }

adminController.safeEquals was added to this codebase for exactly this and is not used here.

And the responses distinguish "no such account" from "wrong code":

// verifyOtpController
if (!user) return res.status(404).send({ message: "User not found", success: false });
if (user.otp !== otp || ...) return res.status(400).send({ message: "Invalid or expired OTP", ... });

// resetPasswordController
if (!user) return res.status(404).send({ message: "User not found", success: false });

forgotPasswordController in the same file already answers uniformly — "If that email exists, an OTP/reset token has been sent." — so the project has already decided that address disclosure matters here. The other two endpoints did not get the same treatment, and either of them will confirm or deny any address in one unauthenticated request.

Expected result

• A stolen copy of the users collection does not let the holder complete a verification or a password reset.
• /verify-otp and /reset-password answer the same way for an address that exists and one that does not.
• A code is compared in constant time and is single-use.
• Guessing a 6-digit code against one account is bounded by something other than patience.

Actual result

• otp and resetToken sit in the database in the exact form the endpoint accepts.
• POST /api/user/verify-otp {"email":"x@y.z","otp":"000000"} returns 404 "User not found" for an unregistered address and 400 "Invalid or expired OTP" for a registered one. /reset-password does the same. Both are unauthenticated and unthrottled.
• !== on a secret is not constant time.
• Nothing counts failed attempts per account. [Security]: Login, OTP and password-reset endpoints accept unlimited attempts, so 6-digit codes are brute-forceable #63 proposes an IP-based limiter, which bounds a single source; it does not bound 10⁶ guesses spread across addresses against one account, and the codes live for ten minutes.
• verifyOtpController returns early with success: true when user.isVerified, without clearing an otp that may still be live on the document.
• resetPasswordController sets user.isVerified = true as a side effect. Holding the reset code does prove control of the mailbox so this is defensible, but it is undocumented and it means the register → verify-OTP path can be skipped entirely by requesting a password reset.

Steps to reproduce

1. Register a@example.com and do not verify it.
2. db.users.findOne({email:"a@example.com"}, {otp:1, otpExpiry:1}) → the six digits, readable, with ten minutes left on them.
3. curl -s -X POST localhost:5000/api/user/verify-otp -H 'Content-Type: application/json' -d '{"email":"a@example.com","otp":"000000"}' → 400 Invalid or expired OTP.
4. Same call with "email":"nobody@example.com" → 404 User not found. The status code is the oracle.
5. Loop step 3 with all 10⁶ codes from as many source addresses as you like — nothing on the account side stops it, and a hit verifies the address.
6. POST /api/user/forgot-password for a verified account, then read resetToken out of the collection and complete /reset-password with it.

Notes

• bcryptjs is already a dependency and is already used for passwords. Hashing the code on write and comparing the candidate against the hash removes the at-rest credential without changing the flow the user sees.
• Storing an attempt counter next to the expiry gives a per-account bound that is independent of, and complementary to, the IP-based throttling in [Security]: Login, OTP and password-reset endpoints accept unlimited attempts, so 6-digit codes are brute-forceable #63 — the two solve different halves of the same problem and neither replaces the other.
• The invalidation rule wants to be uniform: a code dies when it is used, when it expires, when it is superseded by a new one, and when the attempt limit is reached.
• Uniform responses have to stay uniform in timing too, or the absence of a bcrypt compare becomes the new oracle.