bitcoindevkit / bitcoindevkit/bdk_wallet

Wallet signing may finalize invalid pre-seeded signatures and return Ok(true)

Open
#542 1 comment 0 reactions 0 assignees View on GitHub
bug good first issue
Dominant language
Rust
Stars
59
Forks
105
Avg merge
10d 9h
Merged PRs (30d)
1

Description

**Describe the bug**

`Wallet::sign_with_signers` skips signing when the PSBT already has a signature in the wallet key's slot, without verifying it. With default `SignOptions` (`try_finalize: true`), `finalize_psbt` consumes that signature, builds the final witness, and returns `Ok(true)`, although the transaction fails script validation.

Skip sites in `src/wallet/signer.rs`:

- ECDSA: `partial_sigs.contains_key(&pubkey)`
- Taproot key path: `tap_key_sig.is_none()`
- Taproot script path: `tap_script_sigs.contains_key(...)`

**To Reproduce**

1. Create a single-input `wpkh` PSBT.
2. Insert an invalid parseable signature for the wallet pubkey into `partial_sigs`.
3. `wallet.sign_with_signers(&mut psbt, &signers, SignOptions::default())` → `Ok(true)`, invalid sig lands in `final_script_witness`.

Same for Taproot: pre-seed `tap_key_sig` or `tap_script_sigs`.

**Expected behavior**

Overwrite wallet-controlled slots (or verify and overwrite invalid ones) on all three paths. Never `Ok(true)` with an invalid final witness.

**Build environment**
- BDK tag/commit: `6fc68462` (master)
- Rust/Cargo version: 1.96.0

**Which backend(s) are relevant (if any)?**
- [ ] Electrum
- [ ] Esplora
- [ ] Bitcoin Core RPC
- [x] None / not backend-related (e.g. `bdk_chain`, `bdk_core`)
- [ ] Other (please specify): `____`

**Is this blocking production use?**
- [ ] Yes
- [x] No

**Additional context**

`finalize_psbt` checks signature availability, not validity. The `allow_all_sighashes` guard is also bypassed: it checks the PSBT-level `sighash_type` field, not the sighash byte in an existing `partial_sigs` entry.

Contributor guide

Open the contributing guide

Research direction

Start in src/wallet/signer.rs and inspect the ECDSA partial_sigs, Taproot key-path tap_key_sig, and Taproot script-path tap_script_sigs skip sites. Reproduce the invalid pre-seeded-signature cases for wpkh and Taproot, then verify that wallet-controlled invalid signatures are replaced and sign_with_signers does not return Ok(true) with an invalid final witness.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cryptography
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.