wallet: don't abort on crafted MuSig2 PSBT inputs #35154

pull ESultanik wants to merge 1 commits into bitcoin:master from trail-of-forks:security/fix-signmusig2-psbt-assert changing 1 files +6 −1

ESultanik commented at 8:35 PM on April 24, 2026: none
Summary

SignMuSig2() in src/script/sign.cpp derives an attacker-supplied aggregate pubkey along an attacker-supplied BIP32 path (both from untrusted PSBT fields PSBT_IN_TAP_BIP32_DERIVATION and PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS) and then Assert()s the result matches the script pubkey. A mismatched derivation or a hardened index aborts bitcoind via finalizepsbt, analyzepsbt, or descriptorprocesspsbt, with no wallet loaded.

Two crash vectors:
- A hardened index (bit 31 set) in the path hits assert((nChild >> 31) == 0) in CPubKey::Derive (pubkey.cpp).
- A non-hardened path that derives to the wrong pubkey hits Assert(XOnlyPubKey(extpub.pubkey) == script_pubkey) at sign.cpp:313.
Assert() uses std::abort() in all builds (NDEBUG is #error'd in check.h), so this terminates the process.

Impact
- Attack surface: authenticated RPC only; not P2P-reachable.
- No wallet required: finalizepsbt, analyzepsbt, and descriptorprocesspsbt all reach the vulnerable code via DUMMY_SIGNING_PROVIDER. analyzepsbt makes even a read-only inspection a crash trigger.
- Gate is trivial: the only precondition is a 4-byte fingerprint match between two attacker-supplied PSBT fields.
- Affected versions: v31.0rc1, v31.0rc2, and master. Not present in any GA release. Introduced in commit 4a273edda0 (PR #29675). This is release-blocking for v31.0 GA.
- Severity: DoS — instant node abort. No memory corruption, no key or fund compromise.
Reproduction

On master (2d5ab09f0d at the time of testing):
```
bitcoind -regtest -disablewallet -daemon
# Variant A — non-hardened path, hits sign.cpp:313 Assert
bitcoin-cli -regtest finalizepsbt "cHNidP8BAF4CAAAAAaqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqAAAAAAAAAAAAAbiCAQAAAAAAIlEgeb5mfvncu6xVoGKVzocLBwKb/NstzijZWfKBWxb4F5gAAAAAAAEBK6CGAQAAAAAAIlEgeb5mfvncu6xVoGKVzocLBwKb/NstzijZWfKBWxb4F5ghFnm+Zn753LusVaBilc6HCwcCm/zbLc4o2VnygVsW+BeYCQAGr9RrAAAAAAEXIHm+Zn753LusVaBilc6HCwcCm/zbLc4o2VnygVsW+BeYIhoCxgR/lEHtfW0wRUBulcB82Fx3jkuM7zynq6wJuVxwnuUhAvkwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5AAA="
# Variant B — hardened index m/0', hits pubkey.cpp:343 assert
bitcoin-cli -regtest finalizepsbt "cHNidP8BAF4CAAAAAaqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqAAAAAAAAAAAAAbiCAQAAAAAAIlEgeb5mfvncu6xVoGKVzocLBwKb/NstzijZWfKBWxb4F5gAAAAAAAEBK6CGAQAAAAAAIlEgeb5mfvncu6xVoGKVzocLBwKb/NstzijZWfKBWxb4F5ghFnm+Zn753LusVaBilc6HCwcCm/zbLc4o2VnygVsW+BeYCQAGr9RrAAAAgAEXIHm+Zn753LusVaBilc6HCwcCm/zbLc4o2VnygVsW+BeYIhoCxgR/lEHtfW0wRUBulcB82Fx3jkuM7zynq6wJuVxwnuUhAvkwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5AAA="
```
Both PSBTs pass decodepsbt cleanly and would not look suspicious in a MuSig2 / coinjoin workflow. bitcoind aborts on each call; bitcoin-cli reports "EOF reached". analyzepsbt with the same PSBTs also aborts.

Fix

Two small changes in SignMuSig2() at src/script/sign.cpp:
1. Reject hardened derivation indices before calling CPubKey::Derive. Public-key derivation only supports non-hardened steps; a hardened index is attacker-controlled input that can never produce a valid derivation, so skipping the aggregate is correct.
2. Replace the fatal Assert(XOnlyPubKey(extpub.pubkey) == script_pubkey) with if (...) continue;. A pubkey mismatch means this aggregate is not the one that produced script_pubkey. The correct behavior is to move on to the next candidate, not to abort the process.
The diff is 6 added / 1 removed lines.

Credit

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

script: fix reachable Assert() abort in SignMuSig2 via crafted PSBT

Attacker-controlled PSBT fields (PSBT_IN_TAP_BIP32_DERIVATION path and
PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS) flow through a trivially-satisfied
4-byte fingerprint check into CPubKey::Derive() and a fatal Assert().

Two crash vectors exist:
- A hardened derivation index (bit 31 set) in the path hits
  assert((nChild >> 31) == 0) in CPubKey::Derive (pubkey.cpp).
- A mismatched (but unhardened) path derives to the wrong pubkey,
  hitting Assert(XOnlyPubKey(extpub.pubkey) == script_pubkey)
  in SignMuSig2 (sign.cpp).

Both are reachable from any PSBT-processing RPC (finalizepsbt,
analyzepsbt, descriptorprocesspsbt) without a wallet.

Fix: reject hardened indices before entering the derivation loop,
and replace the fatal Assert with a graceful continue that skips
the mismatched aggregate pubkey.

6904dba4f5

DrahtBot commented at 8:35 PM on April 24, 2026: contributor


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



Reviews

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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #35011 (ci, iwyu: Fix warnings in src/script and treat them as errors by BrandonOdiwuor)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
ESultanik renamed this:
~~Don't abort on crafted MuSig2 PSBT inputs~~
cli: don't abort on crafted MuSig2 PSBT inputs
on Apr 24, 2026
DrahtBot added the label Scripts and tools on Apr 24, 2026
ESultanik renamed this:
~~cli: don't abort on crafted MuSig2 PSBT inputs~~
wallet: don't abort on crafted MuSig2 PSBT inputs
on Apr 24, 2026
achow101 commented at 8:48 PM on April 24, 2026: member

This is release-blocking for v31.0 GA.

The release is already out, and this would not be a blocker for it.

Stop calling RPC crashes vulnerabilities.

in src/script/sign.cpp:306 in 6904dba4f5

 301 | @@ -301,6 +302,10 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigda
 302 |              if (!std::equal(agg_info.fingerprint, agg_info.fingerprint + sizeof(agg_info.fingerprint), keyid.data())) {
 303 |                  continue;
 304 |              }
 305 | +            // Reject hardened derivation indices (public key derivation only supports unhardened)
 306 | +            if (std::any_of(agg_info.path.begin(), agg_info.path.end(), [](uint32_t idx) { return idx >> 31; })) {

achow101 commented at 8:53 PM on April 24, 2026:

Instead of looping through the path again, we can do this check below prior to derivation.

Sjors commented at 4:14 PM on April 28, 2026: member

Stop calling RPC crashes vulnerabilities.

It would be nice if a PSBT co-signer service doesn't need to restart their node any time a PSBT from an untrusted source does something unexpected. Perhaps we can add fuzz coverage to enforce that CHECK_NONFATAL is hit, rather than Assert.

Contributors

Labels

Linked (view graph)

#29675 wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys

wallet: don't abort on crafted MuSig2 PSBT inputs #35154

Summary

Impact

Reproduction

Fix

Credit

Reviews

Conflicts