p2p: Compute checksum when decoding v1 messages
What the developer wrote
Authored by rustaceanrob
p2p: Compute checksum when decoding v1 messages
The `sha2_checksum` function is updated to take a `T: Encodable`, which
is passed to `hashes::encode_to_engine`. Two calls to `sha2_checksum`
are unwound, as `Vec<u8>` is not `Encodable`, but both of these
callsites will be removed shortly when `CheckedData` is removed along
with the legacy encoding traits.
What changed, and why it matters
This commit fixes a missing security check in the Bitcoin peer-to-peer message decoder. Previously, when receiving v1 Bitcoin network messages, the code decoded the payload but did not verify the 4-byte checksum in the message header against the actual payload data. That meant corrupted or tampered messages could be accepted as valid. The change makes the decoder compute the expected checksum and reject messages where the checksum does not match.
Treat this as a security fix and include it in the next maintenance release. Users running P2P networking code from affected versions should upgrade. No immediate workaround is described in the commit.
Security signals we found
Missing integrity check on decoded P2P v1 message payloads
Addition of checksum mismatch rejection with explicit error variant
Refactoring of sha2_checksum to operate on Encodable types
Legacy CheckedData checksum paths preserved but marked for removal
Evidence from the diff
The patch adds checksum verification inside V1NetworkMessageDecoder::decode. After the payload is decoded, it calls sha2_checksum(&payload) and compares the result with the checksum parsed from the message header. If they differ, it returns a new InvalidChecksum error. The sha2_checksum helper is refactored to accept &impl encoding::Encodable and use hashes::encode_to_engine, so it can hash the payload directly through the Encodable trait rather than requiring a byte slice. Two legacy call sites in CheckedData are unwound to use sha256d::hash directly because Vec
Changed components
rust-bitcoin p2p/src/message.rsV1NetworkMessageDecodersha2_checksum helperCheckedData (legacy, indirect)Inspect captured patch +28 / −6
diff --git a/p2p/src/message.rs b/p2p/src/message.rs
index dcd0a086..ac8fae8f 100644
--- a/p2p/src/message.rs
+++ b/p2p/src/message.rs
@@ -19,7 +19,7 @@ use encoding::{
self, ArrayDecoder, ArrayEncoder, BytesEncoder, CompactSizeEncoder, Decoder2, Encoder2,
SliceEncoder, VecDecoder,
};
-use hashes::sha256d;
+use hashes::{sha256d, HashEngine};
use internals::ToU64 as _;
use io::{self, BufRead, Read, Write};
use primitives::block::{self, HeaderDecoder, HeaderEncoder};
@@ -1548,6 +1548,15 @@ impl encoding::Decoder for V1NetworkMessageDecoder {
..
} => {
let payload = payload_decoder.end()?;
+ let expected_checksum = sha2_checksum(&payload);
+ if checksum != expected_checksum {
+ return Err(V1NetworkMessageDecoderError(
+ V1NetworkMessageDecoderErrorInner::InvalidChecksum {
+ expected: expected_checksum,
+ actual: checksum,
+ },
+ ));
+ }
Ok(V1NetworkMessage {
magic: Magic::from_bytes(magic_bytes),
@@ -2240,7 +2249,8 @@ pub struct CheckedData {
impl CheckedData {
/// Constructs a new `CheckedData` computing the checksum of given data.
pub fn new(data: Vec<u8>) -> Self {
- let checksum = sha2_checksum(&data);
+ let hash = sha256d::hash(data.as_slice()).to_byte_array();
+ let checksum = [hash[0], hash[1], hash[2], hash[3]];
Self { data, checksum }
}
@@ -2275,7 +2285,8 @@ impl Decodable for CheckedData {
let checksum = <[u8; 4]>::consensus_decode_from_finite_reader(r)?;
let opts = ReadBytesFromFiniteReaderOpts { len, chunk_size: encode::MAX_VEC_SIZE };
let data = read_bytes_from_finite_reader(r, opts)?;
- let expected_checksum = sha2_checksum(&data);
+ let hash = sha256d::hash(data.as_slice()).to_byte_array();
+ let expected_checksum = [hash[0], hash[1], hash[2], hash[3]];
if expected_checksum == checksum {
Ok(Self { data, checksum })
} else {
@@ -2319,9 +2330,11 @@ fn read_bytes_from_finite_reader<D: Read + ?Sized>(
}
/// Does a double-SHA256 on `data` and returns the first 4 bytes.
-fn sha2_checksum(data: &[u8]) -> [u8; 4] {
- let checksum = sha256d::hash(data);
- let checksum = checksum.to_byte_array();
+fn sha2_checksum(data: &impl encoding::Encodable) -> [u8; 4] {
+ let mut engine = sha256d::HashEngine::new();
+ hashes::encode_to_engine(data, &mut engine);
+ let hash = engine.finalize();
+ let checksum = hash.to_byte_array();
[checksum[0], checksum[1], checksum[2], checksum[3]]
}
@@ -2525,6 +2538,8 @@ pub mod error {
PayloadTooLarge,
/// Error decoding the message payload.
Payload,
+ /// Message checksum did not match the one reported in the message header.
+ InvalidChecksum { expected: [u8; 4], actual: [u8; 4] },
}
impl fmt::Display for V1NetworkMessageDecoderError {
@@ -2539,6 +2554,11 @@ pub mod error {
V1NetworkMessageDecoderErrorInner::Payload => {
write!(f, "error decoding message payload")
}
+ V1NetworkMessageDecoderErrorInner::InvalidChecksum { expected: ref e, actual: ref a } => write!(
+ f,
+ "invalid checksum: expected {:02x}{:02x}{:02x}{:02x}, actual {:02x}{:02x}{:02x}{:02x}",
+ e[0], e[1], e[2], e[3], a[0], a[1], a[2], a[3],
+ ),
}
}
}
@@ -2550,6 +2570,8 @@ pub mod error {
V1NetworkMessageDecoderErrorInner::Header => None,
V1NetworkMessageDecoderErrorInner::PayloadTooLarge => None,
V1NetworkMessageDecoderErrorInner::Payload => None,
+ V1NetworkMessageDecoderErrorInner::InvalidChecksum { expected: _, actual: _ } =>
+ None,
}
}
}
Why this scored 60/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.