psbt: detect invalid MuSig2 pubkeys in deserialization #34010

1. 
   rkrux commented at 6:49 am on December 5, 2025: contributor

   Throw error while deserializing PSBT if invalid pubkeys are passed as a MuSig2 aggregate or participant.

   Should fix #33999 & #34201 by throwing error at the very start while decoding an invalid PSBT that should subsequently not allow the MuSig2 signing operation to take place, thereby avoiding the crash.

2. DrahtBot added the label Wallet on Dec 5, 2025
3. 
   DrahtBot commented at 6:49 am on December 5, 2025: 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/34010.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	fjahr, achow101

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

4. 
   theStack commented at 2:00 pm on December 5, 2025: contributor

   The line leading to the crash involves the following two constructors https://github.com/bitcoin/bitcoin/blob/b8e66b901d563d7ba6042e6be54d7e48b9fffc83/src/pubkey.h#L256-L260 where cryptographic validity of the pubkey isn’t checked at any point, so I don’t think this PR fixes anything?
5. 
   rkrux commented at 2:08 pm on December 5, 2025: contributor

   Yes, the second constructor appears to be involved. This comment here suggests where the error lies because of an empty aggregate pubkey added in the fuzz input: #29675 (review)

   I believe that skipping the loop iteration before the following line hits should help in fixing IIUC? https://github.com/bitcoin/bitcoin/blob/b8e66b901d563d7ba6042e6be54d7e48b9fffc83/src/script/sign.cpp#L299

6. 
   theStack commented at 2:34 pm on December 5, 2025: contributor

   Oh right, the crash already happens in the std::span{pubkey}.subspan(1, 32) expression (if std::span{pubkey} is empty due to being invalid), before the first constructor is called. It seems we should detect invalid participant pubkeys as early as possible and not allow them to be added to the wallet state in the first place, I suspect a better place to fix this would be DeserializeMuSig2ParticipantPubkeys, where a validity check is currently missing.
7. 
   rkrux commented at 3:34 pm on December 5, 2025: contributor

   I did think about adding it there but decided against it to keep the change related to the crash site. But I don’t feel strongly about it and will update the PR to add the check in PSBT deserialization.
8. fanquake requested review from achow101 on Dec 5, 2025
9. 
   fjahr commented at 11:55 pm on December 5, 2025: contributor

   Could you add a simple test for this?
10. rkrux force-pushed on Dec 8, 2025
11. rkrux renamed this:
    wallet: check for `agg_pub` validity in MuSig2 signing
    psbt: detect invalid MuSig2 pubkeys in deserialization
    on Dec 8, 2025
12. 
    rkrux commented at 2:32 pm on December 8, 2025: contributor

    I suspect a better place to fix this would be DeserializeMuSig2ParticipantPubkeys, where a validity check is currently missing.

    Updated PR to add the check there instead of in the MuSig2 signing flow that occurs later.

    Could you add a simple test for this?

    Added few tests from the fuzz inputs and created one more test so that the test coverage of an existing if condition that appears at the end of DeserializeMuSig2ParticipantPubkeys is not lost due to the newly added check for invalid participant pubkey.

13. 
    achow101 commented at 10:53 pm on December 23, 2025: member

    ACK 1818f7028e592e52a61b57a67e32eacda78553f9
14. psbt: detect invalid MuSig2 pubkeys in deserialization

    Throw error while deserializing PSBT if invalid pubkeys are passed
    as a MuSig2 aggregate or participant.

    5805a8b540
15. rkrux force-pushed on Jan 5, 2026
16. 
    rkrux commented at 10:55 am on January 5, 2026: contributor

    Updated PR to add a test case from issue #34201.
17. maflcko added this to the milestone 31.0 on Jan 5, 2026
18. 
    fjahr commented at 11:42 am on January 5, 2026: contributor

    utACK 5805a8b54083539c4fede52b4b726102dcfc864e
19. 
    achow101 commented at 10:38 pm on January 5, 2026: member

    ACK 5805a8b54083539c4fede52b4b726102dcfc864e
20. achow101 merged this on Jan 5, 2026
21. achow101 closed this on Jan 5, 2026

    ---

22. 
    theStack commented at 11:22 pm on January 5, 2026: contributor

    post-merge ACK 5805a8b54083539c4fede52b4b726102dcfc864e

Contributors
rkrux DrahtBot theStack fjahr achow101

Review Requested
achow101

Labels
Wallet

Milestone
31.0