Skip to content

Commit f0840c0

Browse files
committed
Merge #232: fix: Validate prevouts in get_psbt_input
6ffc874 fix!: Check prevout bounds in `Wallet::build_fee_bump` (志宇) 00a57cf fix: Validate prevouts in `get_psbt_input` (ItshMoh) Pull request description: Fixes #50 Fixes #51 Replaces bitcoindevkit/bdk#1911 Replaces bitcoindevkit/bdk#1913 ### Description We should check the bounds of the prev txs. ### Changelog notice ```md Fixed: - Wallet::get_psbt_input method now checks bounds when fetching prevout - Wallet::build_fee_bump method now checks bounds when fetching prevout ``` ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing #### Bugfixes: ~~* [ ] I've added tests to reproduce the issue which are now passing~~ * [x] I'm linking the issue being fixed by this PR ACKs for top commit: ValuedMammal: ACK 6ffc874 Tree-SHA512: f2df49b5bc827583251b1dc8b965a038a239a7fa321cc30504d2f1862523be4e132066643c1e1309122d19eefd65ddbac4c714724899b662c6a879df71bd3752
2 parents 29a7374 + 6ffc874 commit f0840c0

File tree

2 files changed

+23
-10
lines changed

2 files changed

+23
-10
lines changed

wallet/src/wallet/error.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,8 @@ pub enum BuildFeeBumpError {
223223
IrreplaceableTransaction(Txid),
224224
/// Node doesn't have data to estimate a fee rate
225225
FeeRateUnavailable,
226+
/// Input references an invalid output index in the previous transaction
227+
InvalidOutputIndex(OutPoint),
226228
}
227229

228230
impl fmt::Display for BuildFeeBumpError {
@@ -247,6 +249,9 @@ impl fmt::Display for BuildFeeBumpError {
247249
write!(f, "Transaction can't be replaced with txid: {}", txid)
248250
}
249251
Self::FeeRateUnavailable => write!(f, "Fee rate unavailable"),
252+
Self::InvalidOutputIndex(op) => {
253+
write!(f, "A txin referenced an invalid output: {}", op)
254+
}
250255
}
251256
}
252257
}

wallet/src/wallet/mod.rs

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,15 +1690,19 @@ impl Wallet {
16901690
.ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))
16911691
// Get chain position
16921692
.and_then(|prev_tx| {
1693-
chain_positions
1693+
let chain_position = chain_positions
16941694
.get(&txin.previous_output.txid)
16951695
.cloned()
1696-
.ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))
1697-
.map(|chain_position| (prev_tx, chain_position))
1696+
.ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))?;
1697+
let prev_txout = prev_tx
1698+
.output
1699+
.get(txin.previous_output.vout as usize)
1700+
.ok_or(BuildFeeBumpError::InvalidOutputIndex(txin.previous_output))
1701+
.cloned()?;
1702+
Ok((prev_tx, prev_txout, chain_position))
16981703
})
1699-
.map(|(prev_tx, chain_position)| {
1700-
let txout = prev_tx.output[txin.previous_output.vout as usize].clone();
1701-
match txout_index.index_of_spk(txout.script_pubkey.clone()) {
1704+
.map(|(prev_tx, prev_txout, chain_position)| {
1705+
match txout_index.index_of_spk(prev_txout.script_pubkey.clone()) {
17021706
Some(&(keychain, derivation_index)) => (
17031707
txin.previous_output,
17041708
WeightedUtxo {
@@ -1708,7 +1712,7 @@ impl Wallet {
17081712
.unwrap(),
17091713
utxo: Utxo::Local(LocalOutput {
17101714
outpoint: txin.previous_output,
1711-
txout: txout.clone(),
1715+
txout: prev_txout.clone(),
17121716
keychain,
17131717
is_spent: true,
17141718
derivation_index,
@@ -1729,10 +1733,10 @@ impl Wallet {
17291733
outpoint: txin.previous_output,
17301734
sequence: txin.sequence,
17311735
psbt_input: Box::new(psbt::Input {
1732-
witness_utxo: txout
1736+
witness_utxo: prev_txout
17331737
.script_pubkey
17341738
.witness_version()
1735-
.map(|_| txout.clone()),
1739+
.map(|_| prev_txout.clone()),
17361740
non_witness_utxo: Some(prev_tx.as_ref().clone()),
17371741
..Default::default()
17381742
}),
@@ -2211,8 +2215,12 @@ impl Wallet {
22112215

22122216
let prev_output = utxo.outpoint;
22132217
if let Some(prev_tx) = self.indexed_graph.graph().get_tx(prev_output.txid) {
2218+
// We want to check that the prevout actually exists in the tx before continuing.
2219+
let prevout = prev_tx.output.get(prev_output.vout as usize).ok_or(
2220+
MiniscriptPsbtError::UtxoUpdate(miniscript::psbt::UtxoUpdateError::UtxoCheck),
2221+
)?;
22142222
if desc.is_witness() || desc.is_taproot() {
2215-
psbt_input.witness_utxo = Some(prev_tx.output[prev_output.vout as usize].clone());
2223+
psbt_input.witness_utxo = Some(prevout.clone());
22162224
}
22172225
if !desc.is_taproot() && (!desc.is_witness() || !only_witness_utxo) {
22182226
psbt_input.non_witness_utxo = Some(prev_tx.as_ref().clone());

0 commit comments

Comments
 (0)