Fix buffer bug in encoding::decode_from_read_unbuffered_with
What the developer wrote
Authored by Mitchell Bagot
Fix buffer bug in encoding::decode_from_read_unbuffered_with
In decode_from_read_unbuffered_with, the clamped buffer is pushed into
the decoder after a successful read. Some decoders do not completely
consume all provided bytes in a single push_bytes call, which can lead
to parts of the buffer being dropped. Like encoding::decode_from_hex,
the push_bytes should instead be called in a loop until the buffer is
empty, or the error case occurs.
Call push_bytes in a loop to prevent buffer bug in
encoding::decode_from_read_unbuffered_with.
What changed, and why it matters
This commit fixes a bug where a Bitcoin data decoder could silently ignore parts of incoming data. The decoder only processed each chunk of data once, but some decoders don't finish a chunk in one go. Leftover bytes were effectively dropped, which could cause malformed or incomplete data to be accepted as valid. The fix repeatedly feeds the remaining bytes until the chunk is fully consumed or an error occurs.
Review whether any released versions shipped with this behavior and assess if malformed inputs could bypass validation or cause consensus-relevant parsing differences. Consider adding regression tests that exercise decoders which consume input incrementally.
Security signals we found
Data truncation / silent dropping of unconsumed decoder input
Inconsistent handling between `decode_from_read_unbuffered_with` and `decode_from_hex`
Potential acceptance of incomplete or malformed consensus-encoded data
Evidence from the diff
In encoding::decode_from_read_unbuffered_with, after a successful read, the clamped buffer was passed once to decoder.push_bytes. Because push_bytes may not consume all provided bytes in a single call (it returns a readiness indicator, not a consumption count), any unconsumed tail of &clamped_buffer[..bytes_read] was discarded. The patch loops on the remaining slice to_push until it is empty or the decoder signals it is ready, matching the behavior already present in encoding::decode_from_hex.
Changed components
consensus_encoding/src/decode/mod.rsencoding::decode_from_read_unbuffered_withInspect captured patch +5 / −6
diff --git a/consensus_encoding/src/decode/mod.rs b/consensus_encoding/src/decode/mod.rs
index a90257fd..a1629ee7 100644
--- a/consensus_encoding/src/decode/mod.rs
+++ b/consensus_encoding/src/decode/mod.rs
@@ -434,12 +434,11 @@ where
return decoder.end().map_err(ReadError::Decode);
}
Ok(bytes_read) => {
- if decoder
- .push_bytes(&mut &clamped_buffer[..bytes_read])
- .map_err(ReadError::Decode)?
- .is_ready()
- {
- return decoder.end().map_err(ReadError::Decode);
+ let mut to_push = &clamped_buffer[..bytes_read];
+ while !to_push.is_empty() {
+ if decoder.push_bytes(&mut to_push).map_err(ReadError::Decode)?.is_ready() {
+ return decoder.end().map_err(ReadError::Decode);
+ }
}
}
Err(ref e) if e.kind() == std::io::ErrorKind::Interrupted => {
Why this scored 61/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.