Open
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
chakra-guy
force-pushed
the
cyfrin/wapi-1121
branch
from
0137247 to
6b2d671
Compare
adonesky1
reviewed
Feb 27, 2026
adonesky1
reviewed
Feb 27, 2026
adonesky1
reviewed
Feb 27, 2026
chakra-guy
added a commit
that referenced
this pull request
Feb 27, 2026
Address review feedback on #69: - Wrap storage.confirmNonce() in try/catch inside the transport's callback lambda so a transient KV store failure emits an error instead of silently dropping a successfully decrypted message. - Move pendingNonces cleanup outside the try block so it always runs regardless of storage success/failure. - Expand MAX_NONCE_JUMP doc comment to explain why skipping the check for new senders is safe (spoofed messages fail decryption and never get confirmed).
chakra-guy
added a commit
that referenced
this pull request
Feb 27, 2026
Address review feedback on #69: - Wrap storage.confirmNonce() in try/catch inside the transport's callback lambda so a transient KV store failure emits an error instead of silently dropping a successfully decrypted message. - Move pendingNonces cleanup outside the try block so it always runs regardless of storage success/failure. - Expand MAX_NONCE_JUMP doc comment to explain why skipping the check for new senders is safe (spoofed messages fail decryption and never get confirmed).
chakra-guy
force-pushed
the
cyfrin/wapi-1121
branch
from
8b37a40 to
a117dc7
Compare
…ryption (WAPI-1121) Nonces are no longer persisted immediately on message receipt. Instead, a confirmNonce callback is emitted with each message and called by BaseClient only after successful decryption. This prevents attackers from poisoning the nonce tracker with high-nonce messages that fail decryption, which would permanently block legitimate messages. - Add MAX_NONCE_JUMP (100) to reject suspiciously large nonce jumps - Add in-memory pendingNonces set to prevent duplicate processing - Add NaN recovery in nonce storage - Add mutex around confirmNonce to prevent race conditions - Wrap confirmNonce in try/catch so storage failures don't drop messages
chakra-guy
force-pushed
the
cyfrin/wapi-1121
branch
from
a117dc7 to
fd3a662
Compare
jiexi
reviewed
Mar 2, 2026
| await this.nonceMutex.runExclusive(async () => { | ||
| const latestNonces = await this.getLatestNonces(channel); | ||
| const current = latestNonces.get(clientId) || 0; | ||
| if (nonce > current) { |
Member
There was a problem hiding this comment.
if this isn't true, hasn't something gone very wrong?
jiexi
reviewed
Mar 2, 2026
| const value = await this.kvstore.get(key); | ||
| const currentNonce = value ? parseInt(value, 10) : 0; | ||
| let currentNonce = value ? parseInt(value, 10) : 0; | ||
| if (Number.isNaN(currentNonce)) currentNonce = 0; |
jiexi
reviewed
Mar 2, 2026
| // If message.nonce <= latestNonce, it's a duplicate and we ignore it. | ||
|
|
||
| // Reject suspiciously large nonce jumps (but allow first message from a new sender). | ||
| if (latestNonce > 0 && message.nonce - latestNonce > MAX_NONCE_JUMP) { |
Member
There was a problem hiding this comment.
I don't quite understand what the max nonce jump is supposed to guard against. Is there a scenario where we think an attacker would send single messages with artificially high nonces? If so, what's the difference between than and simply spamming small incremental nonce bumps?
Could you help me understand the reasoning for always accepting the first seen nonce from the client?
Thank you! sorry if these are obvious
jiexi
reviewed
Mar 2, 2026
Comment on lines
+282
to
+283
| // Without this, a message arriving via both live publication and | ||
| // _fetchHistory could pass the storage-based dedup check twice |
Member
There was a problem hiding this comment.
should handleIncoming message be behind a mutex instead of the pendingNonces guarding logic?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Prevents nonce poisoning attacks by deferring nonce persistence until after successful decryption. Previously, nonces were saved immediately on message receipt, allowing an attacker to send high-nonce messages that fail decryption but permanently block legitimate messages.
BaseClientMAX_NONCE_JUMP(100) to reject suspiciously large nonce jumps from known sendersNaNrecovery in nonce storageconfirmNonceto prevent concurrent race conditionsJira
Test plan
confirmNonce, NaN recovery, nonce regression preventionNote
Medium Risk
Changes transport-level deduplication/nonce persistence behavior and introduces a new
confirmNoncehandshake between transport and client; mistakes could drop or permanently block legitimate messages (e.g., missed confirmations or nonce-jump threshold).Overview
Hardens inbound message handling against nonce-poisoning by deferring persistence of received nonces until after successful decryption:
WebSocketTransportnow emitsmessageevents with aconfirmNonce()callback, andBaseClientcalls it only whendecryptMessagesucceeds.Adds additional nonce safety rails: rejects suspicious large nonce jumps from known senders (
MAX_NONCE_JUMP), tracks pending nonces to avoid double-processing before confirmation, clears pending state onclear(), and makes nonce storage more robust viaNaNrecovery plus a mutex-protectedconfirmNoncepath. Tests and changelog are updated accordingly.Written by Cursor Bugbot for commit fd3a662. This will update automatically on new commits. Configure here.