What the developer wrote
Authored by Mitchell Bagot
Make Witness FromIterator infallible
The current Witness FromIterator impl uses the WitnessDecoder
internally. This was initially done as a means of reducing the
allocations needed to construct the type. However, for DoS protection,
the WitnessDecoder has various limitations that make it unsuitable for
infallible Witness construction. Instead, the old infallible solution
should be used, but tweaked to remove the previous per-item allocations
from the iterator collection.
Adjust the Witness FromIterator implementation to use a collect and
from_slice call to remove the incorrect function beyond the DoS
protection limits of the WitnessDecoder.
What changed, and why it matters
This commit fixes a bug in how a Bitcoin data type called Witness is built from lists of byte slices. The previous code reused a decoder that has built-in denial-of-service limits, which could cause valid-looking witness lists to fail construction unexpectedly. The change makes the construction straightforward and infallible again, removing the risk that normal inputs hit those limits.
Review whether any callers relied on the decoder's DoS limits being enforced during FromIterator construction; if so, add explicit validation. Otherwise, include this fix in the next release and consider adding regression tests for large witness inputs constructed via FromIterator.
Security signals we found
Removal of expect() panic path in infallible trait implementation
DoS protection limits in decoder no longer applied during FromIterator construction
Use of infallible from_slice constructor instead of fallible decoder
Potential panic vector eliminated for large but otherwise valid witness inputs
Evidence from the diff
The FromIterator implementation for Witness previously constructed a raw encoded witness buffer and fed it into WitnessDecoder, which applies DoS protection limits (e.g., maximum element count, maximum total bytes). Because FromIterator is an infallible trait, any input that exceeded those decoder limits would panic via expect(). The patch replaces that path with a simple collect into a Vec and a call to Witness::from_slice, which is the intended infallible constructor. The change also removes the unused Decoder import.
Changed components
primitives/src/witness.rsWitness::from_iter implementationWitnessDecoder usageInspect captured patch +4 / −27
diff --git a/primitives/src/witness.rs b/primitives/src/witness.rs
index b13b21f6..16d23db7 100644
--- a/primitives/src/witness.rs
+++ b/primitives/src/witness.rs
@@ -12,8 +12,7 @@ use arbitrary::{Arbitrary, Unstructured};
#[cfg(doc)]
use encoding::Decoder4;
use encoding::{
- self, BytesEncoder, CompactSizeDecoder, CompactSizeEncoder, Decoder as _, DecoderStatus,
- Encoder2,
+ self, BytesEncoder, CompactSizeDecoder, CompactSizeEncoder, DecoderStatus, Encoder2,
};
#[cfg(feature = "hex")]
use hex::DecodeVariableLengthBytesError;
@@ -659,30 +658,8 @@ impl<'a> IntoIterator for &'a Witness {
impl<T: AsRef<[u8]>> FromIterator<T> for Witness {
fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
- let mut decoder = WitnessDecoder::new();
-
- // We can't count the number of witness elements without consuming the iterator.
- // So instead, we build up the full push_bytes buffer and then push it all at once.
- // We'll start with a 256 byte buffer to double the initial WitnessDecoder size.
- let mut buffer = Vec::with_capacity(256);
- let mut witness_elements = 0;
-
- // For each witness element, the decoder expects an element length, followed by
- // the data itself. The iterator's yielded elements do not include the length prefix,
- // so we add them.
- for elem in iter {
- let encoded = crate::compact_size_encode(elem.as_ref().len());
- buffer.extend_from_slice(encoded.as_slice());
- buffer.extend_from_slice(elem.as_ref());
- witness_elements += 1;
- }
-
- let witness_count = crate::compact_size_encode(witness_elements);
- let _ = decoder.push_bytes(&mut witness_count.as_slice());
-
- let _ = decoder.push_bytes(&mut buffer.as_slice());
-
- decoder.end().expect("witness_elements in decoder is equal to number of provided elements")
+ let v: Vec<T> = iter.into_iter().collect();
+ Self::from_slice(&v)
}
}
@@ -1013,7 +990,7 @@ mod test {
use encoding::check_encode;
#[cfg(feature = "alloc")]
- use encoding::Decode as _;
+ use encoding::{Decode as _, Decoder as _};
use super::*;
Why this scored 47/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.