p2p: Fix ExactSizeEncoder impl on CommandStringEncoder
What the developer wrote
Authored by Mitchell Bagot
p2p: Fix ExactSizeEncoder impl on CommandStringEncoder
The ExactSizeEncoder trait is supposed to return the size of the
remaining data in the encoder. That is, as chunks are encoded and
returned to the caller, .len() should decrease, eventually reaching 0.
The ExactSizeEncoder impl for CommandStringEncoder uses a hard-coded
length, preventing this behaviour.
Change ExactSizeEncoder impl on CommandStringEncoder to call through
to the inner ArrayEncoder impl.
What changed, and why it matters
This commit fixes a small but real bug in a Bitcoin peer-to-peer networking library written in Rust. A helper object that encodes message command names (like "version") was incorrectly reporting that it always had 12 bytes left to send, even after some bytes had already been consumed. The fix makes it report the actual remaining length. The bug could mislead callers that rely on the remaining-length promise, but the commit does not show a direct path to stealing funds or remote code execution.
Treat as a low-severity correctness fix. Review any callers that use ExactSizeEncoder::len() for allocation, progress tracking, or framing decisions to confirm they were not misbehaving before the fix. No urgent patch rollout is indicated unless such a caller is found.
Security signals we found
Violation of a documented trait contract (ExactSizeEncoder)
Incorrect remaining-length reporting in an encoder used for P2P message framing
Potential for buffer-size or progress-checking logic elsewhere to make wrong decisions based on stale length
No direct memory-safety issue: Rust type system still bounds the actual encoded output
Evidence from the diff
CommandStringEncoder wraps an ArrayEncoder for a fixed 12-byte command string. Its ExactSizeEncoder::len() previously returned the constant 12 instead of delegating to the inner encoder. That violated the ExactSizeEncoder contract, which requires len() to decrease to 0 as data is encoded. The patch changes the implementation to self.0.len() and adds a regression test verifying the length starts at 12 and ends at 0 after flush_to_vec.
Changed components
rust-bitcoin p2p/src/message.rsCommandStringEncoderExactSizeEncoder trait implementationInspect captured patch +16 / −1
diff --git a/p2p/src/message.rs b/p2p/src/message.rs
index 37beea1a..a86f3506 100644
--- a/p2p/src/message.rs
+++ b/p2p/src/message.rs
@@ -178,7 +178,7 @@ impl encoding::Encoder for CommandStringEncoder {
impl encoding::ExactSizeEncoder for CommandStringEncoder {
#[inline]
- fn len(&self) -> usize { 12 }
+ fn len(&self) -> usize { self.0.len() }
}
/// Decoder for [`CommandString`].
@@ -2670,4 +2670,19 @@ mod test {
let enc = serialize(&decoded);
assert_eq!(data.as_slice(), enc.as_slice());
}
+
+ #[test]
+ fn command_string_encoder() {
+ use encoding::{Encodable as _, ExactSizeEncoder as _};
+
+ let cmd = CommandString::try_from_static("version").unwrap();
+ let expected_bytes: [u8; 12] = [b'v', b'e', b'r', b's', b'i', b'o', b'n', 0, 0, 0, 0, 0];
+
+ let mut encoder = cmd.encoder();
+ assert_eq!(encoder.len(), expected_bytes.len());
+
+ let encoded = encoding::flush_to_vec(&mut encoder);
+ assert_eq!(encoder.len(), 0);
+ assert_eq!(encoded, expected_bytes);
+ }
}
Why this scored 26/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.