Skip to content

Commit d270735

Browse files
committed
fix(chain): check/sanitize descriptor has not hardened child steps before insert it
1 parent 1afa439 commit d270735

File tree

3 files changed

+59
-2
lines changed

3 files changed

+59
-2
lines changed

crates/chain/src/indexer/keychain_txout.rs

+40-2
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,17 @@ use crate::{
1010
DescriptorExt, DescriptorId, Indexed, Indexer, KeychainIndexed, SpkIterator,
1111
};
1212
use alloc::{borrow::ToOwned, vec::Vec};
13-
use bitcoin::{Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxOut, Txid};
13+
use bitcoin::{
14+
bip32::ChildNumber, Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxOut, Txid,
15+
};
1416
use core::{
1517
fmt::Debug,
1618
ops::{Bound, RangeBounds},
1719
};
20+
use miniscript::{
21+
descriptor::{DescriptorXKey, Wildcard},
22+
ForEachKey,
23+
};
1824

1925
use crate::Merge;
2026

@@ -355,6 +361,23 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
355361
keychain: K,
356362
descriptor: Descriptor<DescriptorPublicKey>,
357363
) -> Result<bool, InsertDescriptorError<K>> {
364+
// Ensure the keys don't contain any hardened derivation steps or hardened wildcards
365+
let descriptor_contains_hardened_steps = descriptor.for_any_key(|k| {
366+
if let DescriptorPublicKey::XPub(DescriptorXKey {
367+
derivation_path,
368+
wildcard,
369+
..
370+
}) = k
371+
{
372+
return *wildcard == Wildcard::Hardened
373+
|| derivation_path.into_iter().any(ChildNumber::is_hardened);
374+
}
375+
false
376+
});
377+
if descriptor_contains_hardened_steps {
378+
return Err(InsertDescriptorError::HardenedDerivationXpub);
379+
}
380+
descriptor.sanity_check()?;
358381
let did = descriptor.descriptor_id();
359382
if !self.keychain_to_descriptor_id.contains_key(&keychain)
360383
&& !self.descriptor_id_to_keychain.contains_key(&did)
@@ -776,7 +799,7 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
776799
}
777800
}
778801

779-
#[derive(Clone, Debug, PartialEq)]
802+
#[derive(Debug, PartialEq)]
780803
/// Error returned from [`KeychainTxOutIndex::insert_descriptor`]
781804
pub enum InsertDescriptorError<K> {
782805
/// The descriptor has already been assigned to a keychain so you can't assign it to another
@@ -793,6 +816,16 @@ pub enum InsertDescriptorError<K> {
793816
/// The descriptor that the keychain is already assigned to
794817
existing_assignment: Descriptor<DescriptorPublicKey>,
795818
},
819+
/// Miniscript error
820+
Miniscript(miniscript::Error),
821+
/// The descriptor contains hardened derivation steps on public extended keys
822+
HardenedDerivationXpub,
823+
}
824+
825+
impl<K> From<miniscript::Error> for InsertDescriptorError<K> {
826+
fn from(err: miniscript::Error) -> Self {
827+
InsertDescriptorError::Miniscript(err)
828+
}
796829
}
797830

798831
impl<K: core::fmt::Debug> core::fmt::Display for InsertDescriptorError<K> {
@@ -816,6 +849,11 @@ impl<K: core::fmt::Debug> core::fmt::Display for InsertDescriptorError<K> {
816849
"attempt to re-assign keychain {keychain:?} already assigned to {existing:?}"
817850
)
818851
}
852+
InsertDescriptorError::Miniscript(err) => write!(f, "Miniscript error: {}", err),
853+
InsertDescriptorError::HardenedDerivationXpub => write!(
854+
f,
855+
"The descriptor contains hardened derivation steps on public extended keys"
856+
),
819857
}
820858
}
821859
}

crates/chain/tests/test_keychain_txout_index.rs

+13
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,19 @@ fn lookahead_to_target() {
600600
}
601601
}
602602

603+
#[test]
604+
fn insert_descriptor_should_reject_hardened_steps() {
605+
use bdk_chain::keychain_txout::KeychainTxOutIndex;
606+
607+
// This descriptor has hardened child steps
608+
let s = "wpkh([e273fe42/84h/1h/0h/0h]tpubDEBY7DLZ5kK6pMC2qdtVvKpe6CiAmVDx1SiLtLS3V4GAGyRvsuVbCYReJ9oQz1rfMapghJyUAYLqkqJ3Xadp3GSTCtdAFcKPgzAXC1hzz8a/*h)";
609+
let (desc, _) =
610+
<Descriptor<DescriptorPublicKey>>::parse_descriptor(&Secp256k1::new(), s).unwrap();
611+
612+
let mut indexer = KeychainTxOutIndex::<&str>::new(10);
613+
assert!(indexer.insert_descriptor("keychain", desc).is_err())
614+
}
615+
603616
#[test]
604617
fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() {
605618
let desc = parse_descriptor(DESCRIPTORS[0]);

crates/wallet/src/wallet/mod.rs

+6
Original file line numberDiff line numberDiff line change
@@ -2547,6 +2547,12 @@ fn create_indexer(
25472547
InsertDescriptorError::KeychainAlreadyAssigned { .. } => {
25482548
unreachable!("this is the first time we're assigning internal")
25492549
}
2550+
InsertDescriptorError::Miniscript(err) => {
2551+
crate::descriptor::error::Error::Miniscript(err)
2552+
}
2553+
InsertDescriptorError::HardenedDerivationXpub => {
2554+
crate::descriptor::error::Error::HardenedDerivationXpub
2555+
}
25502556
}
25512557
})?);
25522558
}

0 commit comments

Comments
 (0)