parallelchain-io / hotstuff_rs

Rust implementation of the HotStuff consensus algorithm.
38 stars 5 forks source link

`DataLen` is being serialized as a `usize`, while being deserialized as a `u32` #57

Closed lyulka closed 1 month ago

lyulka commented 1 month ago

Affected version

v0.4 (branches: dev/v0.4 and hotstuff_rs_spec)

Problem

Because Data::len returns usize, WriteBatch::set_block currently serializes and writes a Data's length into the block tree's backing database as a usize.

Conversely however, KVGet::block_data_len uses DataLen's derived implementation of BorshDeserialize to deserialize a Data's length from the backing database.

Because DataLen is a newtype wrapper around u32, this effectively means that DataLen is being serialized as a u64 (in 64-bit systems), but deserialized as a u32.

This bug does not cause a panic because BorshDeserialize::deserialize does not try to consume the full &mut &[u8] that is provided to it, but merely consumes what it needs and then updates the reference to point the remaining bytes, but causes the deserialized value to be wrong, and so is a severe correctness bug.

Proposed solution

Return DataLen from Data::len instead of usize.