Skip to content

Password reset and email verification use a 4-digit OTP with no per-account attempt limiting #232

Description

@dzekojohn4

Problem

Account-recovery and email-verification OTPs are trivially brute-forceable. UserHelper.generateVerificationCode (backend/src/auth/helper/user-helper.ts) defaults to 4 digits: generateVerificationCode(digits: number = 4) produces one of 10,000 values, valid for 10 minutes (AuthService.createUser, requestResetPasswordOtp, resendVerificationOtp, resendResetPasswordVerificationOtp in backend/src/auth/auth.service.ts all use moment().add(10, 'minutes')).

The recovery endpoints are @Public(): POST /api/auth/verify-otp, verify-reset-password-otp, and reset-password (backend/src/auth/auth.controller.ts). The only protection is the global ThrottlerGuard (per-IP, 100 req/min for the long limiter defined in app.module.ts). There is no per-email or per-OTP attempt counter, lockout, or backoff anywhere in the auth flow. Worse, AuthService.resetPassword looks up the user by OTP alone:

// backend/src/auth/auth.service.ts
const user = await this.userRepository.findOneBy({
  passwordResetCode: otp,   // <-- no email binding; ResetPasswordDto has no email field
});

ResetPasswordDto (backend/src/auth/dto/reset-password.dto.ts) contains only otp, newPassword, confirmNewPassword. Consequence: 10,000 combinations over a 10-minute window, sprayed across many IPs (the per-IP limit is useless against a botnet), is a realistic account-takeover vector — and the OTP doubles as the sole proof of account ownership for the reset.

Root cause

// backend/src/auth/helper/user-helper.ts
public generateVerificationCode(digits: number = 4): string {  // <-- 10k combos
  const max = Math.pow(10, digits) - 1;
  const min = Math.pow(10, digits - 1);
  return (Math.floor(Math.random() * (max - min + 1)) + min).toString();
}

// backend/src/auth/auth.service.ts — resetPassword binds no account
const user = await this.userRepository.findOneBy({ passwordResetCode: otp });

Why this is architecturally hard

  1. Attempt tracking needs storage and a policy. A per-account failed-attempt counter (column on User or a cache key) with lockout/backoff must be added without breaking legitimate resends and multi-device flows. The AccessAttempts counter that exists in contracts/access_control is on-chain and irrelevant to the HTTP API.
  2. The fix spans every OTP flow, not just reset. Verification (verify-otp), reset verification, and reset all share the same 4-digit generator. Raising to 6+ digits and adding attempt limits must be consistent across all of them, including the resend endpoints (which themselves are an abuse surface — they mint a new OTP per call).
  3. resetPassword must bind the OTP to an account the caller controls. Adding email to ResetPasswordDto and verifying it matches the OTP holder changes the API contract (the frontend flow in frontend/app/(auth)/reset-password/page.tsx must send it) — a coordinated backend+frontend change.
  4. Reuse the composite limiter. The per-user + per-IP composite rate-limiting guard already exists (backend/src/common/guards/composite-rate-limit.guard.ts, backend/src/common/decorators/rate-limit.decorator.ts) and is the natural mechanism — wiring it onto these routes with per-email keys is the design decision.

Proposed design (not prescriptive)

  • Raise the default to 6 digits (generateVerificationCode without an override) and keep the 10-minute expiry, or move to a timed single-use token for the reset-link path.
  • Add a failed-attempt counter per email/account (DB column or Redis) with exponential lockout, cleared on success.
  • Apply the composite rate-limit decorator to verify-otp, verify-reset-password-otp, reset-password, and both resend endpoints, keyed by email + IP.
  • Add email to ResetPasswordDto and have resetPassword require a match between the submitted email and the OTP holder.

Acceptance criteria

Contract

  • Generated OTPs are at least 6 digits across verification and reset flows.
  • ResetPasswordDto requires an email that must match the account whose passwordResetCode is presented; the frontend reset flow sends it.

Service

  • Failed OTP attempts against a given email/account are counted and trigger a lockout with backoff; successful verification clears the counter.
  • All OTP endpoints (verify-otp, verify-reset-password-otp, reset-password, resend-verification-otp, resend-reset-password-otp, send-reset-password-otp) are protected by the composite per-email + per-IP limiter.

Tests

  • A brute-force simulation (many wrong OTPs for one email) reaches lockout before exhausting the key space; distributed-IP simulation across accounts cannot exceed the per-email limit.
  • Legit resend-after-lockout behavior is defined and tested (e.g. resend resets the counter but not faster than the rate limit).

Documentation

  • The OTP/lockout policy is documented (e.g. in docs/THREAT-MODEL.md).

Out of scope

WebAuthn/passkey replacement of OTP flows, and rate limiting for /auth/login (already fuzz-tested separately).

Getting started

cd backend
npm install
npm run build
npm test

Good first files to read: backend/src/auth/helper/user-helper.ts, backend/src/auth/auth.service.ts, backend/src/auth/auth.controller.ts, backend/src/auth/dto/reset-password.dto.ts, backend/src/common/guards/composite-rate-limit.guard.ts, and frontend/app/(auth)/reset-password/page.tsx.


Complexity: High — 8 pts

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardThird CampaignCampaign: Third Campaignarea:authImported from local backlog 2026-07-16area:securityImported from local backlog 2026-07-16bugSomething isn't workingpriority:criticalImported from local backlog 2026-07-16

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions