Skip to content

[Security] medium: Email-verification bypass: confirmation token is bound to userID, not the email being confirmed, letting an attacker confirm an email address they do not control #477

Description

@Kravalg

Summary

The email-confirmation token carries only a random tokenValue and the owning userID — it is NOT bound to the specific email address it was issued for (ConfirmationToken has no email field; UserConfirmedEvent only carries tokenValue). ConfirmUserProcessor looks the token up by value, and ConfirmUserCommandHandler loads the user by token->getUserID() and calls User::confirm(), which sets confirmed=true on the user's CURRENT email with no comparison to the address the token was originally sent to. Separately, User::update()/processNewEmail() lets the account owner change their email to any address (only oldPassword + format/uniqueness are checked), sets confirmed=false, and asks EmailChangedEventSubscriber to mail a fresh token to the NEW address. Because a previously issued token (kept by the attacker, e.g. the original registration token, 24h Redis TTL, never invalidated until the async UserConfirmedEvent subscriber deletes it) still resolves to the same userID, replaying it confirms whatever email the account now holds. The email-ownership check that confirmation exists to enforce is therefore fully bypassable for the new address.

Severity: MEDIUM • Category: Business-logic / broken flow enforcement (A04:2021 Insecure Design; CWE-620 Unverified Password/State Change; CWE-841 Improper Enforcement of Behavioral Workflow)
Found by an authorized automated adversarial pentest loop and confirmed by 2 independent skeptic verifiers (unanimous).

Affected code / location

src/User/Domain/Entity/ConfirmationToken.php:28-31 (token holds only tokenValue + userID); src/User/Application/CommandHandler/ConfirmUserCommandHandler.php:31-47; src/User/Domain/Entity/User.php:137-146 (confirm() sets confirmed=true unconditionally) and :260-281 (processNewEmail sets confirmed=false on email change); src/User/Application/EventSubscriber/EmailChangedEventSubscriber.php:35 (issues a NEW token but never invalidates the old one); config/packages/security.yaml:5 (/api/users/confirm PATCH is PUBLIC_ACCESS)

Exploit scenario

  1. Attacker registers with attacker@evil.com; receives confirmation token T1 in their own inbox (userID=U, confirmed=false). Do NOT click it. 2) Authenticated as their own account, attacker PATCHes their profile (UpdateUser) changing email to ceo@victim-corp.com — allowed with just their own current password; this sets confirmed=false and mails a new token T2 to ceo@victim-corp.com (attacker cannot read it). T1 still lives in Redis, keyed by its token value. 3) Attacker submits PATCH /api/users/confirm with token=T1 (public endpoint). ConfirmUser resolves T1 -> userID U -> User::confirm() -> confirmed=true. 4) The account now shows email=ceo@victim-corp.com with confirmed=true, exposed via the API (UserTransformer), even though the attacker never controlled that mailbox. Any downstream trust decision keyed on the 'confirmed'/verified-email flag (impersonation, SSO/JIT provisioning, 'verified sender', privilege gates) is defeated.

Suggested remediation

Bind the confirmation token to the exact email (or a normalized-email hash) it was issued for and verify, inside ConfirmUserCommandHandler/User::confirm(), that token.email === user.currentEmail before flipping confirmed=true; reject otherwise. Additionally invalidate/rotate all outstanding confirmation tokens for a user whenever the email changes (delete the old tokenValue entry in EmailChangedEventSubscriber), and make confirmation single-use synchronously (delete the token in the same transaction as confirm, not via an async domain-event subscriber).

Verification notes

  • Verifier 1 (CONFIRMED, exploitable=true, duplicate=false): Traced the full path in current code. ConfirmationToken.php:28-31 holds only tokenValue+userID (no email). ConfirmUserDto carries only token. ConfirmUserProcessor.php:40 does find(tokenValue) -> ConfirmUserCommandHandler.php:33-39 loads user by token->getUserID() -> User::confirm() (User.php:143) sets confirmed=true unconditionally, with zero comparison to the address the token was issued for. RedisTokenRepository::save() (lines 40-47) stores each token under TWO independent keys, token-tokenValue- and token-userID-; issuing a new token on email change only overwrites the userID key, leaving token-tokenValue- live for its 24h TTL. EmailChangedEventSubscriber.php:35 creates a fresh token but never deletes the old one; the old token is deleted only on successful confirmation (UserConfirmedEventSubscriber). Thus find(T1) still resolves to userID U after the email change. Reachability confirmed: /api/users/confirm PATCH is PUBLIC_ACCESS (security.yaml:5); email is mutable via User::processNewEmail (User.php:260-281) with only oldPassword+format/uniqueness checks, immediately setting the new email and confirmed=false; no signin/PATCH gate on confirmed exists (the sole isConfirmed() gate is in OAuthUserResolver:84, an unrelated auto-link path), so an unconfirmed attacker can authenticate and PATCH their own account. Replaying the stale token flips confirmed=true on an address the attacker never controlled; the confirmed flag is exposed via UserTransformer and OpenAPI response factories, defeating the email-ownership guarantee confirmation exists to provide. Only precondition is that the target address has no existing account (uniqueness), which is the realistic case for confirming an unowned mailbox. Not a duplicate: [Security] high: Password-reset and email-confirmation tokens stored in plaintext at rest #313 was plaintext token storage, [Security] high: Facebook OAuth account takeover via forged emailVerified=true auto-link #318 was OAuth emailVerified auto-link, [Security] medium: Password-reset request endpoint: user-enumeration timing oracle and weak rate-limiting/normalization #321 was reset-timing enumeration; this is a distinct missing token-to-email binding / CWE-841 broken-workflow. Medium severity is appropriate.
  • Verifier 2 (CONFIRMED, exploitable=true, duplicate=false): Every link in the claimed chain is present in the code. ConfirmationToken.php:28-31 stores only tokenValue+userID (no email field); UserConfirmedEvent only carries tokenValue. ConfirmUserProcessor.php:40 looks the token up by value; ConfirmUserCommandHandler.php:33-34 loads the user solely via $token->getUserID(); User::confirm() (User.php:143) sets $this->confirmed=true unconditionally with no comparison to the email the token was issued for. User::processNewEmail() (User.php:260-281) lets the owner change email to any address after only oldPassword validation (UpdateUserCommandHandler.php:37/123-130 verifies the user's OWN password, no mailbox-ownership proof), sets confirmed=false, and fires EmailChangedEvent. EmailChangedEventSubscriber.php:35 issues a fresh token but never invalidates prior tokens. The critical detail — survival of the old token — is confirmed by RedisTokenRepository: save() writes under both a tokenValue-keyed and a userID-keyed entry (lines 40-47), so issuing T2 overwrites only the userID- key while T1's tokenValue- key persists for its 24h TTL (line 24); find() (line 50) resolves by tokenValue, so replaying T1 still returns userID U. The confirm endpoint is PUBLIC_ACCESS (security.yaml:5). Net effect: an attacker confirms confirmed=true on an email address they never controlled, defeating the ownership check confirmation exists to enforce (CWE-841/CWE-620). Not mitigated by any KNOWN item: [Security] high: Password-reset and email-confirmation tokens stored in plaintext at rest #313 addresses plaintext-vs-hashed token storage (an orthogonal property; this binding flaw is independent of storage format), and [Security] critical: OAuth2/JWT access tokens silently escalate to ROLE_SERVICE; issuer/audience validation skipped; session-revocation bypassed #312/[Security] high: Facebook OAuth account takeover via forged emailVerified=true auto-link #318/[Security] medium: Broken object-level authorization: any authenticated user can read and enumerate all users' PII #320-[Security] low: Account-lockout failure counter uses non-atomic read-modify-write (race allows extra guesses) #324 address unrelated flows. Reachable and exploitable in present code.

Related

Part of the enterprise security-hardening effort — umbrella tracker #348; security NFR issues #426 (securability), #441 (vulnerability), #362 (confidentiality), #389 (integrity). Prior security wave: #312#324. Not a duplicate of the already-fixed items in that range.


Acceptance = the exploit path is closed AND a regression test (unit/Behat/Schemathesis) proves it stays closed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendphpPull requests that update Php codesecuritySecurity vulnerability

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions