SEP-10 challenge TOCTOU race allows double JWT issuance

[DeFiVC]

What

The verifyChallenge method checks redis.exists() at line 81 and only deletes the challenge at line 150, after a long processing window (XDR decoding, signature verification, database queries). Two concurrent verify requests for the same address can both pass the exists check before either deletes the key. Both then successfully verify and both get JWTs issued.

Why

An attacker can obtain multiple JWTs from a single SEP-10 challenge, enabling token multiplication.

Scope

• Use an atomic check-and-delete operation (e.g., Redis GETDEL or a Lua script)
• Ensure the challenge is consumed exactly once

Acceptance Criteria

• Two concurrent verify requests for the same challenge only succeed once
• The challenge is atomically consumed (checked + deleted in one operation)

Technical Context

• File: src/modules/auth/auth.service.ts, lines 81-150
• Current flow: redis.exists() → verify → redis.del()
• Fix: Use redis.getdel() or a Lua script for atomic check-and-delete

[AbelOsaretin]

Hello maintainer.

I'm interested in this issue because TOCTOU race conditions in authentication flows are exactly the kind of subtle concurrency bug I enjoy fixing — especially when it enables token multiplication, which is a real security risk. The fact that this lives in the SEP-10 challenge verification path (the gateway to all authenticated functionality) makes it high-impact.

I've worked with Redis atomic operations extensively in TypeScript backends, including GETDEL, SETNX patterns, and Lua scripts for complex multi-step atomics. I've patched similar race conditions in auth services where check-then-act patterns were inadvertently left non-atomic. I'm also familiar with the Stellar SDK and ioredis, which this codebase uses.

My approach is straightforward: replace the non-atomic redis.exists() (line 81) + redis.del() (line 150) pair with a single redis.getdel() call near the top of verifyChallenge. This returns the challenge data and deletes the key atomically — only the first concurrent request will succeed. If getdel() returns null, throw the existing "Challenge expired or not found" error. No Lua script needed since ioredis supports getDel natively, and it keeps the diff minimal and easy to review.

[grantfox-oss]

🦊 GrantFox — @AbelOsaretin has been assigned to this issue as part of the Official Campaign | FWC26 campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #72)
2. Your PR will be reviewed by the ChainLearnOfficial maintainers

Good luck! Track your progress on GrantFox.

[grantfox-oss]

🎉 This issue has been marked as completed on GrantFox as part of the Official Campaign | FWC26 campaign!

@AbelOsaretin's PR #84 was approved and merged by @DeFiVC.

🏆 @AbelOsaretin: You earned 35 FoxPoints for this contribution! Your current tier: Explorer (662 total points). Track your full progress on GrantFox.

👏 Great work, @AbelOsaretin! Keep contributing to ChainLearnOfficial.