fix: InstructionIndices::nth byte position bug
What the developer wrote
Authored by Alexey Gradoboev
fix: InstructionIndices::nth byte position bug
Remove the custom `nth` implementation for `InstructionIndices` because
it captures position via `next_with` and returns as byte offset instead of advancing it in later next_fn calls.
Fixes #6377
What changed, and why it matters
This commit fixes a bug in a Rust Bitcoin library iterator that reports byte positions of script instructions. The custom `nth` method incorrectly returned the byte offset of the found instruction without updating the iterator's internal position, so later calls to advance the iterator would report wrong positions. Removing the custom `nth` lets the normal iterator machinery keep positions correct. The practical security impact is limited because it is a correctness bug in reporting indices, not a direct memory-safety or cryptographic flaw, but code relying on these indices could make wrong decisions.
Treat as a correctness fix with low-to-moderate security relevance. Review any code that calls `.nth()` on `InstructionIndices` and then continues iteration, as prior indices would have been wrong. No immediate emergency response is warranted unless downstream components use these indices for security-sensitive bounds or validation decisions.
Security signals we found
Iterator state corruption in a public API
Incorrect byte-index reporting for Bitcoin script instructions
Potential downstream logic errors if indices are used for parsing or validation
No direct memory safety, cryptographic, or consensus-critical signal in the diff
Evidence from the diff
The InstructionIndices iterator wraps Instructions and tracks the current byte offset (pos) as it yields (byte_index, Instruction) pairs. The removed nth override called self.next_with(|this| this.instructions.nth(n)). next_with adds the instruction’s byte length to pos after each yielded item, but when instructions.nth(n) skips n items internally, only one position update happens for the returned item, not for the n skipped items. Consequently, after calling nth, pos lags behind the actual byte offset, corrupting all subsequent index values. The fix removes the override so Iterator::nth’s default implementation calls next() repeatedly, which correctly advances pos for every skipped instruction.
Changed components
bitcoin/src/blockdata/script/instruction.rsInstructionIndices iteratorScript instruction parsing/indexing consumersInspect captured patch +0 / −5
diff --git a/bitcoin/src/blockdata/script/instruction.rs b/bitcoin/src/blockdata/script/instruction.rs
index e7037591..684e5e08 100644
--- a/bitcoin/src/blockdata/script/instruction.rs
+++ b/bitcoin/src/blockdata/script/instruction.rs
@@ -253,11 +253,6 @@ impl<'a> Iterator for InstructionIndices<'a> {
#[inline]
fn size_hint(&self) -> (usize, Option<usize>) { self.instructions.size_hint() }
-
- // the override avoids computing pos multiple times
- fn nth(&mut self, n: usize) -> Option<Self::Item> {
- self.next_with(|this| this.instructions.nth(n))
- }
}
impl core::iter::FusedIterator for InstructionIndices<'_> {}
Why this scored 35/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.