[Security][Go-live] Pre-registration email verification codes generated with System.Random (predictable) #440

New Issue

2026-07-06T07:11:26Z

spikerj commented

2026-07-06 07:11:26 +00:00

Problem

The 6-digit EmailVerificationCode that gates pre-registration email ownership is generated with System.Random, in an identical GenerateNumericCode helper duplicated across two handlers:

InitiatePreRegistrationCommandHandler.cs:372 (new Random() → random.Next(0,10) per digit)
ResendPreRegistrationCodesCommandHandler.cs:556 (same)

System.Random is seeded from the system clock and is not cryptographically secure. An attacker who can estimate when a code was issued (registration time is often observable) can reconstruct the seed and predict the code, taking over a pre-registration / verifying an email they don't control. The code space is only 10^6, and while VerifyPreRegistrationEmail does increment EmailVerificationAttempts, prediction defeats any attempt cap (correct on the first try).

Fix

Generate codes with a CSPRNG. Added SpikerSoft.Business.Domain.Authentication.SecureNumericCode.Generate(length) using RandomNumberGenerator.GetInt32(0, 10) per digit, and routed both handlers through it (removed the duplicated new Random() helpers).

Verification

SecureNumericCodeTests: correct length, digits-only, low collision rate across many draws (a clock-seeded new Random() in a tight loop can repeat), non-positive length throws. Build clean.

Related (not in this PR — flagging for the team)

InitiatePreRegistrationCommandHandler.HashPassword stores an unsalted SHA-256 of the password in the pre-reg record ("temporary before Keycloak"). If that store leaks, those are rainbow-table-crackable. Worth deciding whether the password should be stored at all pre-Keycloak, or via a proper salted KDF — likely belongs with #404.
Confirm EmailVerificationAttempts is actually enforced (rejects after N), not just incremented/logged.

Resolved by spikersoft-backend PR (linked below).

## Problem The 6-digit `EmailVerificationCode` that gates pre-registration email ownership is generated with **`System.Random`**, in an identical `GenerateNumericCode` helper duplicated across two handlers: - `InitiatePreRegistrationCommandHandler.cs:372` (`new Random()` → `random.Next(0,10)` per digit) - `ResendPreRegistrationCodesCommandHandler.cs:556` (same) `System.Random` is seeded from the system clock and is **not** cryptographically secure. An attacker who can estimate *when* a code was issued (registration time is often observable) can reconstruct the seed and **predict the code**, taking over a pre-registration / verifying an email they don't control. The code space is only 10^6, and while `VerifyPreRegistrationEmail` does increment `EmailVerificationAttempts`, prediction defeats any attempt cap (correct on the first try). ## Fix Generate codes with a CSPRNG. Added `SpikerSoft.Business.Domain.Authentication.SecureNumericCode.Generate(length)` using `RandomNumberGenerator.GetInt32(0, 10)` per digit, and routed both handlers through it (removed the duplicated `new Random()` helpers). ## Verification `SecureNumericCodeTests`: correct length, digits-only, low collision rate across many draws (a clock-seeded `new Random()` in a tight loop can repeat), non-positive length throws. Build clean. ## Related (not in this PR — flagging for the team) - `InitiatePreRegistrationCommandHandler.HashPassword` stores an **unsalted SHA-256** of the password in the pre-reg record ("temporary before Keycloak"). If that store leaks, those are rainbow-table-crackable. Worth deciding whether the password should be stored at all pre-Keycloak, or via a proper salted KDF — likely belongs with #404. - Confirm `EmailVerificationAttempts` is actually **enforced** (rejects after N), not just incremented/logged. Resolved by spikersoft-backend PR (linked below).

spikerj commented

2026-07-06 14:13:13 +00:00

Resolved in spikersoft-backend PR #130 (merged to master as 389f570). Pre-registration verification codes now use SecureNumericCode (CSPRNG via RandomNumberGenerator) in both Initiate + Resend handlers. 5/5 tests. Closing.

Two adjacent items remain flagged here for the team (not in this PR): the unsalted-SHA-256 HashPassword pre-Keycloak (belongs with #404), and confirming EmailVerificationAttempts is enforced vs. only incremented — reopen/track separately if you want those actioned.

Resolved in spikersoft-backend PR #130 (merged to `master` as `389f570`). Pre-registration verification codes now use `SecureNumericCode` (CSPRNG via `RandomNumberGenerator`) in both Initiate + Resend handlers. 5/5 tests. Closing. Two adjacent items remain flagged here for the team (not in this PR): the unsalted-SHA-256 `HashPassword` pre-Keycloak (belongs with #404), and confirming `EmailVerificationAttempts` is enforced vs. only incremented — reopen/track separately if you want those actioned.

spikerj closed this issue

2026-07-06 14:13:35 +00:00

spikerj referenced this issue

2026-07-20 05:58:32 +00:00

EPIC: Angular 22 + Nx 23 full-leverage modernization (signal forms, resource APIs, affected CI, remote cache) #722

spikerj referenced this issue

2026-07-21 04:17:43 +00:00

Direct messages: role/family-scoped 1:1 messaging with presence (FAB + drawer) #767

Sign in to join this conversation.