psbt: don't abort on invalid MuSig2 derivations #35933

pull l0rinc wants to merge 2 commits into bitcoin:master from l0rinc:l0rinc/musig2-psbt-derivation-abort changing 2 files +28 −4

l0rinc commented at 8:01 PM on August 7, 2026: contributor

Problem: A PSBT may contain MuSig2 derivation metadata with a hardened child index or a path that derives to a different key. The hardened index aborts during public derivation, while the mismatched key aborts at the result assertion. analyzepsbt, finalizepsbt, and descriptorprocesspsbt all reach this code without a wallet. Even the read-only analyzepsbt can force a co-signer service to restart its node after unexpected input.

Fix: Return failure when a MuSig2 derivation path contains a hardened child index, and skip only the current aggregate when the path derives to a different key so another matching aggregate can still be tried.

This follows #35154, with the related contributions credited in the commits.

test: characterize MuSig2 derivation aborts

Exercise `analyzepsbt` and `finalizepsbt` with mismatched and hardened MuSig2 aggregate derivation paths, restarting the node after each abort so the current behavior remains executable.

e3d1e75a51

DrahtBot added the label PSBT on Aug 7, 2026
DrahtBot commented at 8:01 PM on August 7, 2026: contributor



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



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35933.



Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK jeanpablojp

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
in src/script/sign.cpp:325 in 7a3e2e21d1
```
 317 | @@ -318,14 +318,19 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigda
 318 |              }
 319 |              // Get the BIP32 derivation tweaks
 320 |              CExtPubKey extpub = CreateMuSig2SyntheticXpub(agg_pub);
 321 | -            for (const int i : agg_info.path) {
 322 | +            bool hardened_path{false};
 323 | +            for (const uint32_t i : agg_info.path) {
 324 | +                if (i >> 31) {
 325 | +                    hardened_path = true;
 326 | +                    break;
```
achow101 commented at 11:46 PM on August 7, 2026:

In 7a3e2e21d17b4177c399e86ff3ed7d550b63a151 "psbt: skip invalid MuSig2 derivation paths"

Why break instead of return false? If we reach this, the data given in the PSBT is actually invalid and there's nothing that we can do. It's impossible to have hardened derivation with MuSig2 aggregate keys.

Furthermore, because it is invalid, we probably shouldn't continue with attempting to sign.

l0rinc commented at 12:40 AM on August 8, 2026:

Makes sense to return false for invalid (likely malicious) content. Pushed.

Type	Reviewers
ACK	jeanpablojp

psbt: avoid aborting on invalid MuSig2 derivations

`SignMuSig2()` derives MuSig2 aggregate keys from metadata supplied by a PSBT.
A hardened child cannot be publicly derived, and a path that produces another key does not identify the aggregate being signed.

Check each child index as `uint32_t` immediately before deriving that child, preserving the hardened bit without an implementation-defined conversion to `int`.
Return failure if a child index is hardened, and skip a path if it derives to a different key so another aggregate can still be tried.
The descriptor parser enforces the same requirement for MuSig2 aggregate derivations.

Co-authored-by: Anthropic Security <security-cvd@anthropic.com>
Co-authored-by: Evan Sultanik <evan.sultanik@trailofbits.com>
Co-authored-by: Ava Chow <github@achow101.com>

73a94b4545

l0rinc renamed this:
~~psbt: skip invalid MuSig2 derivation paths~~
psbt: don't abort on invalid MuSig2 derivations
on Aug 8, 2026
l0rinc force-pushed on Aug 8, 2026
jeanpablojp commented at 12:18 PM on August 9, 2026: none

ACK 73a94b45459a9433ffcf99aff3b044cc7166ee12

I have tested the code. Reverting only sign.cpp reproduces both aborts here. Built the merge with master (128456b): unit suite and rpc_psbt.py pass.

nit: descriptorprocesspsbt aborts the same way and the fix covers it, but the test only exercises analyzepsbt and finalizepsbt.

Contributors

Labels

Linked (view graph)

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

psbt: don't abort on invalid MuSig2 derivations #35933

Code Coverage & Benchmarks

Reviews