#35984 sign: skip signing SIGHASH_SINGLE inputs with no corresponding output
https://github.com/bitcoin/bitcoin/pull/35984 · · +37/-8 in 2 files, 1 commits · labels: none
Goal
- Prevent wallet users from signing unsafe SIGHASH_SINGLE inputs without a matching output
- Protects users from fund theft if tricked into signing a malicious PSBT
Moves the SIGHASH_SINGLE input bounds check into `MutableTransactionSignatureCreator::CreateSig`. This ensures both standard transaction signing and PSBT signing refuse to sign an input when no corresponding output exists at that index.
Problem: When an input is signed with SIGHASH_SINGLE without a matching output index, legacy and segwit v0 transactions commit to no outputs at all. This creates a valid signature that an adversary can detach and attach to an arbitrary transaction, redirecting funds without the signer's consent.
Category: Wallet (#1 of 84)
P1 · fund safety
- P2 because it prevents a fund-safety vulnerability during PSBT signing
- Protects users from transaction output redirection when handling untrusted PSBTs
- Eliminates an unsafe signing footgun across all wallet signing paths
P1 because producing an output-independent signature is a fund-safety vulnerability. As described by furszy: 'the signature stays valid even when outputs are swapped, which is a footgun that lets funds be redirected without the owner's consent.'
Membership: Fixes PSBT signing logic in `src/script/sign.cpp` used directly by wallet signing RPCs like `walletprocesspsbt`.
Factors: security/stability 2, bug 2, performance 0, user value 2, leverage 0
Reviewability: Ready
- Ready for review with passing CI and no open objections
The change is compact, CI passes cleanly, and there are no unresolved review requests.
Author status: active, rebased recently
Resolved concerns:
- l0rinc asked to link and fix issue #35977, which the author incorporated in the description
Agreement: Strong
- Strong support with no opposition to fixing the signing footgun
- Concept approval for addressing the issue quickly (l0rinc)
- Reviewed and approved an earlier revision (achow101)
Strong consensus with an ACK from achow101 and concept approval from l0rinc
Multiple maintainers and contributors support closing the signature validation loophole, with zero objections.
- achow101: 'ACK 0fa39483fb891d7dd0e35942c927646756550132'
- l0rinc: 'Concept ACK'
Objections: none enumerated.
Support:
- achow101: ACK on code and test changes
- l0rinc: Concept ACK for fixing the dangerous signing gap described in #35977
Participants: l0rinc (support), achow101 (support)
State derived from the lists: substantive support, no open objection (achow101, l0rinc)
Review verdicts (DrahtBot): 0 (+1)
Dependencies
Enables:
- Closes issue #35977
Files
33 lines under test/bench/ci.
- test/functional/rpc_psbt.py +31/-2
- src/script/sign.cpp +6/-6
Card
This PR moves the check skipping SIGHASH_SINGLE signing for inputs without a corresponding output into CreateSig. Previously, SignTransaction skipped these dangerous inputs but SignPSBTInput did not, allowing walletprocesspsbt to produce detached signatures that commit to no outputs and risk fund theft. The PR has an ACK from achow101 and Concept ACK from l0rinc with no open objections, fixes #35977, and is ready for merge.