feat: SIWE-style nonce verification has a TOCTOU race

context/progress-tracker.md

@@ -6,6 +6,41 @@ pure chore/docs commits). Direct pushes to main must also be logged here.
 
 ---
 
+## 2026-08-28
+
+- Fixed TOCTOU nonce reuse in `AuthService.verifySignature()` (src/modules/auth/auth.service.ts:111):
+  - **Atomic nonce consumption** — replaced `SELECT → verify → UPDATE` with atomic
+    conditional claim `UPDATE nonces SET used_at = now() WHERE id = ? AND used_at IS NULL`
+    executed **before** signature verification. Only the winner of the race gets
+    `count === 1` / `data.length === 1`; losers get `count === 0` and are rejected
+    with `AUTH_NONCE_NOT_FOUND`. This guarantees a given `(wallet, nonce)` can
+    produce at most one successful verification ever, even under concurrent
+    `POST /auth/verify` requests carrying the same stolen pair.
+  - **Burn-on-failure tradeoff documented in code** — if verification fails
+    (invalid signature, bad StrKey, or `expires_at` in the past) the nonce stays
+    burned. The caller must request a fresh nonce; this converts replay attacks
+    into DoS-on-self (one wasted challenge) versus unlimited session creation.
+    Chosen over RPC/locking because a single conditional `UPDATE` is natively atomic
+    in Postgres and fits the existing `SupabaseService.getServiceRoleClient()`
+    pattern without a new migration.
+  - **Per-wallet throttling on `POST /auth/verify`** — new `AuthWalletThrottlerGuard`
+    (src/modules/auth/auth-throttler.guard.ts) keys `@nestjs/throttler` on
+    `req.body.wallet` (fallback to `req.user.wallet` / IP) and is applied via
+    `@UseGuards(AuthWalletThrottlerGuard)` alongside the existing global
+    IP-based `ThrottlerGuard`. Route limit stays `5 req / 60 s` per wallet **and**
+    per IP, preventing offline-style brute force of the SEP-0043 fallback space
+    at network speed. `WalletThrottlerGuard` was also hardened to type-check
+    wallet strings and accept `body.wallet` so the same infrastructure is reused.
+  - **Tests** — extended `test/unit/modules/auth/auth.service.spec.ts` to prove
+    atomicity: parallel double-verify → exactly one success, replay after success
+    fails, replay after failure stays burned (`AUTH_SIGNATURE_INVALID` → `AUTH_NONCE_NOT_FOUND`),
+    expired nonce rejected and stays burned, atomic race via `count === 0` rejected.
+    Added `test/unit/modules/auth/auth-throttler.guard.spec.ts` for the wallet-keyed
+    throttler and updated `auth.controller.spec.ts` to mock the guard. `npm run build`
+    and `npm test` green (38 suites, 425 tests).
+
+---
+
 ## 2026-08-27
 
 - Closed the audit gaps on `POST /transactions/submit` (#117):

src/modules/auth/auth-throttler.guard.ts

@@ -0,0 +1,31 @@
+import { Injectable } from '@nestjs/common';
+import { ThrottlerGuard } from '@nestjs/throttler';
+
+/**
+ * ThrottlerGuard variant for POST /auth/verify that keys rate limits on the
+ * wallet address supplied in the request body (unauthenticated) or on the
+ * authenticated wallet (if present). Prefers body wallet because verify is
+ * unauthenticated — the wallet is not yet in req.user.
+ *
+ * Falls back to the default IP-based tracker when no wallet is present so
+ * anonymous/probe traffic is still bounded per IP.
+ *
+ * Used alongside the global IP-based ThrottlerGuard so POST /auth/verify is
+ * bounded per wallet AND per IP — preventing brute-force of the SEP-0043
+ * fallback space at network speed and limiting stolen-nonce replay attempts.
+ */
+@Injectable()
+export class AuthWalletThrottlerGuard extends ThrottlerGuard {
+  protected async getTracker(req: Record<string, unknown>): Promise<string> {
+    const body = (req as { body?: { wallet?: unknown } }).body;
+    const user = (req as { user?: { wallet?: unknown } }).user;
+    const bodyWallet = typeof body?.wallet === 'string' ? body.wallet : undefined;
+    const userWallet = typeof user?.wallet === 'string' ? user.wallet : undefined;
+    // Prefer body wallet for unauthenticated verify; fall back to user wallet.
+    const wallet = bodyWallet ?? userWallet;
+    if (wallet) {
+      return `wallet:${wallet}`;
+    }
+    return super.getTracker(req);
+  }
+}

src/modules/auth/auth.controller.ts

@@ -1,21 +1,23 @@
-import { 
-  Controller, 
-  Post, 
-  Body, 
-  HttpCode, 
-  HttpStatus, 
+import {
+  Controller,
+  Post,
+  Body,
+  HttpCode,
+  HttpStatus,
   NestInterceptor,
   ExecutionContext,
   CallHandler,
-  UseInterceptors, 
-  UploadedFile, 
-  ParseFilePipe, 
-  MaxFileSizeValidator, 
-  FileTypeValidator 
+  UseInterceptors,
+  UseGuards,
+  UploadedFile,
+  ParseFilePipe,
+  MaxFileSizeValidator,
+  FileTypeValidator,
 } from '@nestjs/common';
 import { ApiTags, ApiOperation, ApiResponse, ApiConsumes, ApiBody } from '@nestjs/swagger';
 import { Throttle } from '@nestjs/throttler';
 import { AuthService, RegisterResponse } from './auth.service';
+import { AuthWalletThrottlerGuard } from './auth-throttler.guard';
 import { UploadedAvatarFile } from '../../database/repositories/users.repository';
 import { NonceRequestDto } from './dto/nonce-request.dto';
 import { NonceResponseDto } from './dto/nonce-response.dto';
@@ -73,9 +75,11 @@ export class AuthController {
   @Post('verify')
   @HttpCode(HttpStatus.OK)
   @Throttle({ default: { limit: 5, ttl: 60000 } })
+  @UseGuards(AuthWalletThrottlerGuard)
   @ApiOperation({ summary: 'Verify wallet signature and issue JWT tokens' })
   @ApiResponse({ status: 200, description: 'Signature verified — JWT tokens issued', type: AuthResponseDto })
   @ApiResponse({ status: 401, description: 'Invalid signature or nonce' })
+  @ApiResponse({ status: 429, description: 'Too many requests - rate limit exceeded (per wallet or per IP)' })
   async verify(@Body() dto: VerifyRequestDto): Promise<AuthResponseDto> {
     await this.authService.verifySignature(dto);
     return this.authService.generateTokens(dto.wallet);

src/modules/auth/auth.service.ts

@@ -190,9 +190,47 @@ export class AuthService {
     if (nonceError || !nonceRecord) {
       throw new UnauthorizedException({ code: 'AUTH_NONCE_NOT_FOUND', message: 'Nonce not found or already used.' });
     }
-    if (new Date(nonceRecord.expires_at) < new Date()) {
+
+    // Atomic nonce claim: consume the row BEFORE expensive signature verification.
+    // The conditional UPDATE ... WHERE id = ? AND used_at IS NULL is a single
+    // atomic statement in Postgres. Two concurrent verify requests that both
+    // observed the same unused row above will race here; only one UPDATE will
+    // affect a row (count === 1). The loser gets count === 0 / empty data and
+    // is rejected as already consumed. This eliminates the TOCTOU window that
+    // previously existed between the SELECT and the trailing UPDATE.
+    //
+    // SECURITY TRADEOFF: if signature verification subsequently fails (or the
+    // nonce is expired), the nonce stays burned. Callers must request a fresh
+    // nonce and re-sign. This converts a replay of a stolen nonce+signature
+    // into a DoS-on-self (one wasted challenge) which is the correct tradeoff
+    // versus allowing unlimited session creation from a single intercepted pair.
+    const claimedAt = new Date().toISOString();
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Supabase builder count typings for update().select() chain are incomplete; need runtime count check
+    const claimResult: any = await (client.from('nonces') as any)
+      .update({ used_at: claimedAt }, { count: 'exact' })
+      .eq('id', (nonceRecord as { id: string }).id)
+      .is('used_at', null)
+      .select('id');
+    const claimError = claimResult?.error as { message: string } | null | undefined;
+    const claimData = claimResult?.data as unknown[] | null | undefined;
+    const claimCount = claimResult?.count as number | null | undefined;
+    if (claimError) {
+      throw new InternalServerErrorException({
+        code: 'DATABASE_NONCE_CLAIM_FAILED',
+        message: 'Failed to claim nonce.',
+      });
+    }
+    const claimedCount = typeof claimCount === 'number' ? claimCount : (claimData?.length ?? 0);
+    if (claimedCount === 0) {
+      throw new UnauthorizedException({ code: 'AUTH_NONCE_NOT_FOUND', message: 'Nonce not found or already used.' });
+    }
+
+    // Nonce is now burned regardless of outcome below. Check expiry AFTER the
+    // claim so an expired row is still consumed and cannot be retried.
+    if (new Date((nonceRecord as { expires_at: string }).expires_at) < new Date()) {
       throw new UnauthorizedException({ code: 'AUTH_NONCE_EXPIRED', message: 'Nonce has expired.' });
     }
+
     if (!StrKey.isValidEd25519PublicKey(dto.wallet)) {
       throw new UnauthorizedException({ code: 'AUTH_SIGNATURE_INVALID', message: 'Invalid signature.' });
     }
@@ -229,7 +267,6 @@ export class AuthService {
       if (err instanceof UnauthorizedException) throw err;
       throw new UnauthorizedException({ code: 'AUTH_SIGNATURE_INVALID', message: 'Invalid signature.' });
     }
-    await client.from('nonces').update({ used_at: new Date().toISOString() }).eq('id', nonceRecord.id);
   }
 
   /**

src/modules/transactions/wallet-throttler.guard.ts

@@ -6,11 +6,19 @@ import { ThrottlerGuard } from '@nestjs/throttler';
  * (from the JWT payload) instead of the client IP. Used alongside the global
  * IP-based guard so POST /transactions/submit is bounded per wallet AND per
  * IP, preventing a single wallet from being used as an open relay to Horizon.
+ *
+ * Also checks req.body.wallet so the same guard can be reused for
+ * unauthenticated routes like POST /auth/verify where the wallet is in the
+ * request body.
  */
 @Injectable()
 export class WalletThrottlerGuard extends ThrottlerGuard {
-  protected async getTracker(req: { user?: { wallet?: string } }): Promise<string> {
-    const wallet = req.user?.wallet;
-    return wallet ? `wallet:${wallet}` : super.getTracker(req);
+  protected async getTracker(req: Record<string, unknown>): Promise<string> {
+    const user = (req as { user?: { wallet?: unknown } }).user;
+    const body = (req as { body?: { wallet?: unknown } }).body;
+    const userWallet = typeof user?.wallet === 'string' ? user.wallet : undefined;
+    const bodyWallet = typeof body?.wallet === 'string' ? body.wallet : undefined;
+    const wallet = userWallet ?? bodyWallet;
+    return wallet ? `wallet:${wallet}` : super.getTracker(req as Record<string, unknown>);
   }
 }

test/unit/modules/auth/auth-throttler.guard.spec.ts

@@ -0,0 +1,59 @@
+import { AuthWalletThrottlerGuard } from '../../../../src/modules/auth/auth-throttler.guard';
+
+describe('AuthWalletThrottlerGuard', () => {
+  function createGuard(): AuthWalletThrottlerGuard {
+    const storageService = {
+      increment: jest.fn(),
+      getRecord: jest.fn(),
+    };
+    const options = [{ ttl: 60000, limit: 5 }];
+    const reflector = {};
+    return new (AuthWalletThrottlerGuard as unknown as new (
+      ...args: unknown[]
+    ) => AuthWalletThrottlerGuard)(options, storageService, reflector);
+  }
+
+  function getTrackerOf(guard: AuthWalletThrottlerGuard, req: unknown): Promise<string> {
+    return (
+      guard as unknown as { getTracker: (request: unknown) => Promise<string> }
+    ).getTracker(req);
+  }
+
+  it('keys the rate limit on the wallet from request body when present (unauthenticated verify)', async () => {
+    const guard = createGuard();
+    const wallet = 'GABCDEFGHIJKLMNOPQRSTUVWXYZ234567ABCDEFGHIJKLMNOPQRSTUVW';
+
+    await expect(getTrackerOf(guard, { body: { wallet } })).resolves.toBe(`wallet:${wallet}`);
+  });
+
+  it('keys the rate limit on the authenticated wallet when present', async () => {
+    const guard = createGuard();
+    const wallet = 'GABCDEFGHIJKLMNOPQRSTUVWXYZ234567ABCDEFGHIJKLMNOPQRSTUVW';
+
+    await expect(getTrackerOf(guard, { user: { wallet } })).resolves.toBe(`wallet:${wallet}`);
+  });
+
+  it('prefers body wallet over user wallet when both are present', async () => {
+    const guard = createGuard();
+    const bodyWallet = 'GBODYWALLETAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA';
+    const userWallet = 'GUSERWALLETAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA';
+
+    await expect(
+      getTrackerOf(guard, { body: { wallet: bodyWallet }, user: { wallet: userWallet } }),
+    ).resolves.toBe(`wallet:${bodyWallet}`);
+  });
+
+  it('falls back to the IP-based tracker when no wallet is present', async () => {
+    const guard = createGuard();
+
+    await expect(getTrackerOf(guard, { ip: '203.0.113.7' })).resolves.toBe('203.0.113.7');
+  });
+
+  it('falls back to IP when body wallet is not a string', async () => {
+    const guard = createGuard();
+
+    await expect(getTrackerOf(guard, { ip: '198.51.100.9', body: { wallet: 123 } })).resolves.toBe(
+      '198.51.100.9',
+    );
+  });
+});

test/unit/modules/auth/auth.controller.spec.ts

@@ -1,6 +1,7 @@
 import { Test, TestingModule } from '@nestjs/testing';
 import { AuthController } from '../../../../src/modules/auth/auth.controller';
 import { AuthService } from '../../../../src/modules/auth/auth.service';
+import { AuthWalletThrottlerGuard } from '../../../../src/modules/auth/auth-throttler.guard';
 
 describe('AuthController', () => {
   let controller: AuthController;
@@ -30,7 +31,10 @@ describe('AuthController', () => {
           useValue: mockAuthService,
         },
       ],
-    }).compile();
+    })
+      .overrideGuard(AuthWalletThrottlerGuard)
+      .useValue({ canActivate: jest.fn().mockReturnValue(true) })
+      .compile();
 
     controller = module.get<AuthController>(AuthController);
     authService = module.get<AuthService>(AuthService);