Skip to content

fix(chain): sanitize and bound-check derivation indices near BIP32_MAX_INDEX #2265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
busayo-OD wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix(chain): sanitize and bound-check derivation indices near BIP32_MAX_INDEX #2265

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
803fd96
fix(chain): clamp derivation index before apply_changeset
busayo-OD Sep 6, 2026
22ad1ec
fix(chain): fix off-by-one in replenish_inner_index stop bound
busayo-OD Sep 6, 2026
713586e
fix(chain): fix integer overflow in lookahead_to_target
busayo-OD Sep 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 142 additions & 3 deletions crates/chain/src/indexer/keychain_txout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,8 +563,9 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
pub fn lookahead_to_target(&mut self, keychain: K, target_index: u32) -> ChangeSet {
let mut changeset = ChangeSet::default();
if let Some((next_index, _)) = self.next_index(keychain.clone()) {
let temp_lookahead = (target_index + 1)
.checked_sub(next_index)
let temp_lookahead = target_index
.checked_add(1)
.and_then(|t| t.checked_sub(next_index))
.filter(|&index| index > 0);

if let Some(temp_lookahead) = temp_lookahead {
Expand Down Expand Up @@ -601,7 +602,7 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
// Exclusive: index to stop at.
let stop_index = if descriptor.has_wildcard() {
let next_reveal_index = self.last_revealed.get(&did).map_or(0, |v| *v + 1);
(next_reveal_index + lookahead).min(BIP32_MAX_INDEX)
(next_reveal_index + lookahead).min(BIP32_MAX_INDEX.saturating_add(1))
} else {
1
};
Expand Down Expand Up @@ -992,6 +993,7 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
}
}
for (did, index) in changeset.last_revealed {
let index = index.min(BIP32_MAX_INDEX);

@evanlinjin evanlinjin Sep 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose adding debug_assert!(index < BIP32_MAX_INDEX) above this line so that overflows can be caught during development (it shouldn't really happen).

@busayo-OD busayo-OD Sep 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried the debug_assert! here, but it breaks apply_changeset_clamps_out_of_range_index_and_derives_correct_spk, which passes BIP32_MAX_INDEX + 1 to verify that apply_changeset clamps the value.

I also moved the test inline and changed it to use a wildcard descriptor so it exercises the boundary arithmetic. I added an SPK correctness check at the boundary as well.

let v = self.last_revealed.entry(did).or_default();
*v = index.max(*v);
self.replenish_inner_index_did(did, self.lookahead);
Expand Down Expand Up @@ -1226,4 +1228,141 @@ mod test {
let index = KeychainTxOutIndex::<i32>::from_changeset(lookahead, false, init_cs);
assert!(index.spk_cache.is_empty());
}

#[test]
fn apply_changeset_clamps_out_of_range_index_and_derives_correct_spk() {
let s = DESCRIPTORS[0];
let desc = Descriptor::parse_descriptor(&Secp256k1::new(), s)
.unwrap()
.0;
let mut index = KeychainTxOutIndex::new(0, false);
let did = desc.descriptor_id();
let _ = index.insert_descriptor(0i32, desc.clone());
let seed_index = BIP32_MAX_INDEX - 1;
let spk = desc
.at_derivation_index(seed_index)
.unwrap()
.script_pubkey();
index.inner.insert_spk((0i32, seed_index), spk);
let changeset = ChangeSet {
last_revealed: [(did, BIP32_MAX_INDEX + 1)].into(),
..Default::default()
};
index.apply_changeset(changeset);
assert_eq!(index.last_revealed_index(0i32), Some(BIP32_MAX_INDEX));
assert_eq!(
index.spk_at_index(0i32, BIP32_MAX_INDEX),
Some(
desc.at_derivation_index(BIP32_MAX_INDEX)
.unwrap()
.script_pubkey()
)
);
}

#[test]
fn reveal_next_spk_and_next_unused_spk_return_last_script_when_saturated() {
let s = DESCRIPTORS[0];
let desc = Descriptor::parse_descriptor(&Secp256k1::new(), s)
.unwrap()
.0;
let mut index = KeychainTxOutIndex::new(0, false);
let did = desc.descriptor_id();
let _ = index.insert_descriptor(0i32, desc.clone());
let spk = desc
.at_derivation_index(BIP32_MAX_INDEX)
.unwrap()
.script_pubkey();
index.inner.insert_spk((0i32, BIP32_MAX_INDEX), spk);
index.last_revealed.insert(did, BIP32_MAX_INDEX);
let (i, changeset) = index.reveal_next_spk(0i32).unwrap();
assert_eq!(i.0, BIP32_MAX_INDEX);
assert!(changeset.is_empty());
assert!(index.mark_used(0i32, BIP32_MAX_INDEX));
let (i, changeset) = index.next_unused_spk(0i32).unwrap();
assert_eq!(i.0, BIP32_MAX_INDEX);
assert!(changeset.is_empty());
}

#[test]
fn reveal_to_target_with_target_at_bip32_max_index() {
let s = DESCRIPTORS[0];
let desc = Descriptor::parse_descriptor(&Secp256k1::new(), s)
.unwrap()
.0;
let mut index = KeychainTxOutIndex::new(0, false);
let did = desc.descriptor_id();
let _ = index.insert_descriptor(0i32, desc.clone());

let seed_index = BIP32_MAX_INDEX - 1;
let spk = desc
.at_derivation_index(seed_index)
.unwrap()
.script_pubkey();
index.inner.insert_spk((0i32, seed_index), spk);
index.last_revealed.insert(did, seed_index);

let (spks, _changeset) = index.reveal_to_target(0i32, BIP32_MAX_INDEX).unwrap();
assert_eq!(spks.len(), 1);
assert_eq!(spks[0].0, BIP32_MAX_INDEX);
}

#[test]
fn reveal_to_target_with_target_above_bip32_max_index() {
let s = DESCRIPTORS[0];
let desc = Descriptor::parse_descriptor(&Secp256k1::new(), s)
.unwrap()
.0;
let mut index = KeychainTxOutIndex::new(0, false);
let did = desc.descriptor_id();
let _ = index.insert_descriptor(0i32, desc.clone());

let seed_index = BIP32_MAX_INDEX - 1;
let spk = desc
.at_derivation_index(seed_index)
.unwrap()
.script_pubkey();
index.inner.insert_spk((0i32, seed_index), spk);
index.last_revealed.insert(did, seed_index);

let (spks, _changeset) = index.reveal_to_target(0i32, BIP32_MAX_INDEX + 5).unwrap();
assert_eq!(spks.len(), 1);
assert_eq!(spks[0].0, BIP32_MAX_INDEX);
}

#[test]
fn lookahead_to_target_with_u32_max_does_not_overflow() {
let s = DESCRIPTORS[0];
let desc = Descriptor::parse_descriptor(&Secp256k1::new(), s)
.unwrap()
.0;
let mut index = KeychainTxOutIndex::new(0, false);
let _ = index.insert_descriptor(0i32, desc.clone());

let changeset = index.lookahead_to_target(0i32, u32::MAX);
assert!(changeset.is_empty());
}

#[test]
fn lookahead_to_target_with_target_at_bip32_max_index() {
let s = DESCRIPTORS[0];
let desc = Descriptor::parse_descriptor(&Secp256k1::new(), s)
.unwrap()
.0;
let mut index = KeychainTxOutIndex::new(0, false);
let did = desc.descriptor_id();
let _ = index.insert_descriptor(0i32, desc.clone());

let seed_index = BIP32_MAX_INDEX - 1;
let spk = desc
.at_derivation_index(seed_index)
.unwrap()
.script_pubkey();
index.inner.insert_spk((0i32, seed_index), spk);
index.last_revealed.insert(did, seed_index);

let _changeset = index.lookahead_to_target(0i32, BIP32_MAX_INDEX);

assert!(index.spk_at_index(0i32, BIP32_MAX_INDEX).is_some());
}
}