diff --git a/zebra-chain/src/block.rs b/zebra-chain/src/block.rs index 6f741832e..3713598d0 100644 --- a/zebra-chain/src/block.rs +++ b/zebra-chain/src/block.rs @@ -21,6 +21,7 @@ pub use header::BlockTimeError; pub use header::{CountedHeader, Header}; pub use height::Height; pub use root_hash::RootHash; +pub use serialize::MAX_BLOCK_BYTES; use serde::{Deserialize, Serialize}; diff --git a/zebra-chain/src/serialization/read_zcash.rs b/zebra-chain/src/serialization/read_zcash.rs index 8a06b14ad..0527ace8c 100644 --- a/zebra-chain/src/serialization/read_zcash.rs +++ b/zebra-chain/src/serialization/read_zcash.rs @@ -11,6 +11,15 @@ use super::SerializationError; pub trait ReadZcashExt: io::Read { /// Reads a `u64` using the Bitcoin `CompactSize` encoding. /// + /// # Security + /// + /// Deserialized sizes must be validated before being used. + /// + /// Preallocating vectors using untrusted `CompactSize`s allows memory + /// denial of service attacks. Valid sizes must be less than + /// `MAX_BLOCK_BYTES / min_serialized_item_bytes` (or a lower limit + /// specified by the Zcash consensus rules or Bitcoin network protocol). + /// /// # Examples /// /// ```rust @@ -87,15 +96,6 @@ pub trait ReadZcashExt: io::Read { Ok(SocketAddr::new(ip_addr, port)) } - /// Read a Bitcoin-encoded UTF-8 string. - #[inline] - fn read_string(&mut self) -> Result { - let len = self.read_compactsize()?; - let mut buf = vec![0; len as usize]; - self.read_exact(&mut buf)?; - String::from_utf8(buf).map_err(|_| SerializationError::Parse("invalid utf-8")) - } - /// Convenience method to read a `[u8; 4]`. #[inline] fn read_4_bytes(&mut self) -> io::Result<[u8; 4]> { diff --git a/zebra-chain/src/serialization/zcash_deserialize.rs b/zebra-chain/src/serialization/zcash_deserialize.rs index a7ac07d72..ac9d5425a 100644 --- a/zebra-chain/src/serialization/zcash_deserialize.rs +++ b/zebra-chain/src/serialization/zcash_deserialize.rs @@ -1,6 +1,7 @@ use std::io; use super::{ReadZcashExt, SerializationError}; +use byteorder::ReadBytesExt; /// Consensus-critical serialization for Zcash. /// @@ -33,6 +34,21 @@ impl ZcashDeserialize for Vec { } } +/// Read a byte. +impl ZcashDeserialize for u8 { + fn zcash_deserialize(mut reader: R) -> Result { + Ok(reader.read_u8()?) + } +} + +/// Read a Bitcoin-encoded UTF-8 string. +impl ZcashDeserialize for String { + fn zcash_deserialize(reader: R) -> Result { + let bytes: Vec<_> = Vec::zcash_deserialize(reader)?; + String::from_utf8(bytes).map_err(|_| SerializationError::Parse("invalid utf-8")) + } +} + /// Helper for deserializing more succinctly via type inference pub trait ZcashDeserializeInto { /// Deserialize based on type inference diff --git a/zebra-chain/src/transaction/serialize.rs b/zebra-chain/src/transaction/serialize.rs index 85d374f4b..0efbb0a3e 100644 --- a/zebra-chain/src/transaction/serialize.rs +++ b/zebra-chain/src/transaction/serialize.rs @@ -44,22 +44,24 @@ impl ZcashSerialize for JoinSplitData

{ impl ZcashDeserialize for Option> { fn zcash_deserialize(mut reader: R) -> Result { - let joinsplits: Vec> = Vec::zcash_deserialize(&mut reader)?; - - if joinsplits.is_empty() { - Ok(None) - } else { - let (first, rest) = joinsplits - .split_first() - .expect("a non-empty Vec must have at least one entry"); - let pub_key = reader.read_32_bytes()?.into(); - let sig = reader.read_64_bytes()?.into(); - Ok(Some(JoinSplitData { - first: first.clone(), - rest: rest.to_vec(), - pub_key, - sig, - })) + let num_joinsplits = reader.read_compactsize()?; + match num_joinsplits { + 0 => Ok(None), + n => { + let first = sprout::JoinSplit::zcash_deserialize(&mut reader)?; + let mut rest = Vec::with_capacity((n - 1) as usize); + for _ in 0..(n - 1) { + rest.push(sprout::JoinSplit::zcash_deserialize(&mut reader)?); + } + let pub_key = reader.read_32_bytes()?.into(); + let sig = reader.read_64_bytes()?.into(); + Ok(Some(JoinSplitData { + first, + rest, + pub_key, + sig, + })) + } } } } diff --git a/zebra-network/src/protocol/external/codec.rs b/zebra-network/src/protocol/external/codec.rs index a290e9ed6..96d7611c8 100644 --- a/zebra-network/src/protocol/external/codec.rs +++ b/zebra-network/src/protocol/external/codec.rs @@ -464,7 +464,7 @@ impl Codec { reader.read_socket_addr()?, ), nonce: Nonce(reader.read_u64::()?), - user_agent: reader.read_string()?, + user_agent: String::zcash_deserialize(&mut reader)?, start_height: block::Height(reader.read_u32::()?), relay: match reader.read_u8()? { 0 => false, @@ -488,7 +488,7 @@ impl Codec { fn read_reject(&self, mut reader: R) -> Result { Ok(Message::Reject { - message: reader.read_string()?, + message: String::zcash_deserialize(&mut reader)?, ccode: match reader.read_u8()? { 0x01 => RejectReason::Malformed, 0x10 => RejectReason::Invalid, @@ -501,7 +501,7 @@ impl Codec { 0x50 => RejectReason::Other, _ => return Err(Error::Parse("invalid RejectReason value in ccode field")), }, - reason: reader.read_string()?, + reason: String::zcash_deserialize(&mut reader)?, // Sometimes there's data, sometimes there isn't. There's no length // field, this is just implicitly encoded by the body_len. // Apparently all existing implementations only supply 32 bytes of