wallet: allow re-processing nonce-less MuSig2 PSBT #35155

Summary

SetMuSig2SecNonce() in src/script/signingprovider.cpp calls try_emplace on the persistent per-wallet m_musig2_secnonces map and then Assert(inserted). When walletprocesspsbt is called twice on the same nonce-less MuSig2 PSBT (either a natural retry, or the case where a counterparty strips the victim's pubnonce and returns the PSBT) the deterministic session_id = SHA256(script_pubkey || part_pubkey || sighash) collides, try_emplace returns inserted=false, and the Assert aborts bitcoind.

Impact

• Attack surface: authenticated RPC plus a wallet containing a MuSig2 participant private key.
• Trigger: either (a) user or tooling retries walletprocesspsbt assuming idempotency, or (b) a malicious MuSig2 cosigner returns the PSBT with the victim's pubnonce stripped, prompting the victim's client to re-process.
• No nonce reuse occurs. The Assert is intended to prevent nonce reuse, but regenerating a fresh secnonce (overwriting an unused old one) is cryptographically safe; it invalidates the previously-shared pubnonce, requiring a signing round restart, which is exactly the retry scenario.
• Design conflict: the guidance at src/util/check.h:125-126 recommends CHECK_NONFATAL for conditions reachable from RPC. Assert() uses std::abort() in all builds (NDEBUG is #error'd in check.h).
• Affected versions: v31.0rc1, v31.0rc2, and master. Not present in any GA release. Introduced in commit c9519c260b (PR #33636). Release-blocking for v31.0 GA.
• Severity: DoS — node abort.

Reproduction

On master (2d5ab09f0d at the time of testing):

# In regtest with wallet support:
# 1. create two descriptor wallets (musig_p0, musig_p1)
# 2. import tr(musig(priv0/<0;1>/*,pub1/<2;3>/*)) into wallet 0
# 3. fund the musig address and confirm
# 4. walletcreatefundedpsbt -> nonce-less MuSig2 PSBT
# 5. walletprocesspsbt $PSBT        # first call: complete=false, stores secnonce
# 6. walletprocesspsbt $PSBT        # second call on original PSBT: Assert(inserted), process aborts

Fix

Replace the try_emplace + Assert(inserted) in FlatSigningProvider::SetMuSig2SecNonce with insert_or_assign. Overwriting an unused secnonce is cryptographically safe: the previously returned pubnonce is invalidated, and the session must restart from the new nonce, which is the intent of the retry. Nothing else reads the old secnonce between the two calls.

Credit

Found by Claude during an automated review conducted by Anthropic; manually validated and patched by Trail of Bits. Reference: ANT-2026-05771.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

This PR and #35154 present the issues as vulnerabilities, while they are not.

We do not consider crashes in the RPCs as vulnerabilities, so please change the PR description. Including a functional test would also help with verifying the analysis performed by your LLM.

We do not consider crashes in the RPCs as vulnerabilities

Indeed, though it would be good if (even if nothing else is fixed) this is handled with CHECK_NONFATAL.

From a usability point of view, we should probably:

1. Refuse to start a second signing session unless the first one is explicitly aborted (but how?); or
2. Emit a warning that we started a second signing session (this has tripped me up in my own testing)

cc @achow101

Emit a warning that we started a second signing session (this has tripped me up in my own testing)

We should do that. We should key by the pubnonce as well in order to distinguish multiple sessions over the same transaction.