Skip to content

critical: SIWE-style nonce verification has a TOCTOU race — one nonce can authenticate multiple sessions #116

Description

@EmeditWeb

Problem

AuthService.verifySignature() (src/modules/auth/auth.service.ts, lines 94–142) verifies nonces with a read-then-write pattern:

  1. SELECT ... WHERE used_at IS NULL AND nonce = ? AND wallet = ? (line 96–102)
  2. verify the signature (lines 109–136)
  3. only afterwards UPDATE nonces SET used_at = ... (line 141)

Two concurrent POST /auth/verify requests carrying the same (wallet, nonce, signature) both execute step 1 before either reaches step 3 — Supabase returns the same unused row to both — and both pass. One stolen or intercepted nonce+signature pair therefore mints unlimited independent sessions (each with its own refresh token), defeating the entire point of single-use challenges. This is textbook TOCTOU; the window widens because the expensive signature verification sits between the check and the mark-used.

Additionally there is no cleanup guarantee tying expired-nonce deletion to this flow, and nothing rate-limits verify attempts per wallet, enabling offline-style brute forcing of the SEP-0043 fallback space at network speed.

Ground Rules

  1. Read context/architecture-context.md, context/code-standards.md, context/progress-tracker.md in full
  2. Read src/database/supabase.client.ts and how service-role queries are issued elsewhere
  3. Do not restructure the auth module; fix the claim semantics inside the established flow

What To Build

  1. Make nonce consumption atomic: replace the SELECT→verify→UPDATE sequence with an atomic conditional claim — UPDATE nonces SET used_at = now() WHERE id = ? AND used_at IS NULL executed BEFORE signature verification, treating count === 0 as "nonce consumed/expired". Only proceed to verify the signature if the claim won exactly one row.
  2. If verification then fails, the nonce stays burned — document this tradeoff in code comments (it converts replays into DoS-on-self, which is correct).
  3. Alternatively implement the claim via a Postgres RPC/function with row locking if the client-side update cannot be made atomic — choose whichever the codebase's Supabase access pattern supports cleanly.
  4. Add per-wallet attempt throttling on /auth/verify (reuse any existing throttle infrastructure; if none exists, an in-memory or Redis-backed limiter consistent with repo conventions).
  5. Tests: parallel double-verify yields exactly one success; replay after success fails; replay during failure burns the nonce; expired nonce rejected.

Files To Touch

  • src/modules/auth/auth.service.ts
  • src/modules/auth/auth.controller.ts (throttle wiring)
  • database migration/RPC if chosen approach requires it
  • test files
  • relevant docs/progress tracker

Acceptance Criteria

  • A given (wallet, nonce) can produce at most one successful verification ever
  • Concurrency test proves atomicity under parallel requests
  • Verify endpoint is throttled per wallet
  • Suite green

Mandatory Checks Before Opening PR

Standard checklist: conventions, build, lint, tests, PR template, exact issue reference. PRs failing any check will be closed without review.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions