Skip to content

fix(recovery): do not open an account recovery request unless the owner was notified #497

Closed

Description

@Bccorb

Self-service account recovery (#493, unreleased) relies on its delay plus the "recovery opened" notice: every verified address is told and handed a cancel link. That notice is the only place the cancel token is delivered. The API stores only its hash and never returns it. Today the request opens whether or not any notice actually went out, so on some deployments the owner is never told and cannot cancel.

Where

  • src/services/accountRecoveryService.ts (verifyRecovery): the request is set pending with completesAt before any notice is attempted.
  • src/services/accountNotices.ts:63-79 (sendAccountNotice):
    • Under external delivery it records not_sent_external_delivery and sends nothing.
    • On a DeliveryError it records failed and carries on. This includes MessagingConfigurationError when no direct transport is configured.
  • src/controllers/accountRecovery.ts:201-219 (verifyAccountRecovery): notice outcomes only reach the audit event. The response is 200 either way.
  • src/controllers/accountRecovery.ts:243-262: cancel needs the token. No signed-in owner session can see or cancel a pending request, and a normal sign-in does not cancel it.
  • src/controllers/accountRecovery.ts:124-130: the recovery code is delivered as a plain otp_email / otp_sms kind, so an adapter cannot word it as a recovery warning.

When it bites

  • account_recovery.enabled is on (default off) and phishing_resistant_only is off.
  • The API has no working direct notice transport, or the caller uses external delivery with a service token.
    • The no-transport case is common for adopters who deliver through the adapter's messaging transports.
    • /recovery/start is marked delivery: true, so the code reaches the user that way.
    • /recovery/verify then tries direct delivery for the notices, and every one fails.
    • A provider outage at the wrong moment has the same effect.

Scenario

The attacker has ongoing read access to the victim's email or phone, such as a forwarding rule or a SIM swap. This is the attacker the delay is designed to stop ("It cannot complete quietly", docs/account-recovery.md).

  1. /login, /recovery/start, read the code, /recovery/verify. The request opens; no notice and no cancel link reach the owner.
  2. After the delay: /login, /recovery/start, read the second code, /recovery/verify (resumed, clock unchanged), /recovery/complete. Every owner session is revoked and the attacker holds a recovery session.
  3. The attacker enrolls a passkey, which lifts the restriction.

This matters most on passkey-only accounts, where channel access alone would otherwise not get in.

Fix (before v0.20.0)

  • Fail closed. Keep the request open only when at least one notice to a verified address was actually sent. Otherwise void it and return an error that says why.
  • External delivery:
    • Add a delivery kind for the opened notice, e.g. account_recovery_opened, carrying the cancel URL.
    • Return it from /recovery/verify, and mark that route delivery: true in the adapter manifest.
    • Needs a @seamless-auth/types addition and the adapter passthroughs (see seamless-auth-server#232).
  • Use a distinct delivery kind for the recovery code itself, so adapters can warn that it is a recovery code and not a sign-in code.
  • Defence in depth: list and cancel pending recovery requests from any access session. The docs already name this as a follow-up.
  • Tests covering each fail-closed branch, and docs/account-recovery.md updated.

Found in the v0.20 pre-release security review. It does not affect any released version.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    P1Security work outside the FIDO2 trackbugSomething isn't workingsecuritySecurity-sensitive: auth, tokens, OTP, sessions, cryptov0.20Targets the v0.20 release

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions