Security: reject compact sizes greater than the protocol message limit (#2155)

These sizes should be impossible in valid messages.
So they likely represent a memory preallocation attack.

zebra-chain/src/block/tests/preallocate.rs

@@ -3,7 +3,7 @@
 use crate::{
     block::{
         header::MIN_COUNTED_HEADER_LEN, CountedHeader, Hash, Header, BLOCK_HASH_SIZE,
-        MAX_PROTOCOL_MESSAGE_LEN,
+        MAX_BLOCK_BYTES, MAX_PROTOCOL_MESSAGE_LEN,
     },
     serialization::{TrustedPreallocate, ZcashSerialize},
 };
@@ -51,7 +51,7 @@ proptest! {
     /// Confirm that each counted header takes at least COUNTED_HEADER_LEN bytes when serialized.
     /// This verifies that our calculated `TrustedPreallocate::max_allocation()` is indeed an upper bound.
     #[test]
-    fn counted_header_min_length(header in Header::arbitrary_with(()), transaction_count in (0..std::u32::MAX)) {
+    fn counted_header_min_length(header in Header::arbitrary_with(()), transaction_count in (0..MAX_BLOCK_BYTES)) {
         let header = CountedHeader {
             header,
             transaction_count: transaction_count.try_into().expect("Must run test on platform with at least 32 bit address space"),

zebra-chain/src/serialization/read_zcash.rs

@@ -1,9 +1,12 @@
 use std::io;
-use std::net::{IpAddr, SocketAddr};
+use std::{
+    convert::TryInto,
+    net::{IpAddr, SocketAddr},
+};
 
 use byteorder::{BigEndian, LittleEndian, ReadBytesExt};
 
-use super::SerializationError;
+use super::{SerializationError, MAX_PROTOCOL_MESSAGE_LEN};
 
 /// Extends [`Read`] with methods for writing Zcash/Bitcoin types.
 ///
@@ -17,12 +20,17 @@ pub trait ReadZcashExt: io::Read {
     ///
     /// 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
+    /// `MAX_PROTOCOL_MESSAGE_LEN / min_serialized_item_bytes` (or a lower limit
     /// specified by the Zcash consensus rules or Bitcoin network protocol).
     ///
+    /// As a defence-in-depth for memory preallocation attacks,
+    /// Zebra rejects sizes greater than the protocol message length limit.
+    /// (These sizes should be impossible, because each array items takes at
+    /// least one byte.)
+    ///
     /// # Examples
     ///
-    /// ```rust
+    /// ```
     /// use zebra_chain::serialization::ReadZcashExt;
     ///
     /// use std::io::Cursor;
@@ -41,22 +49,21 @@ pub trait ReadZcashExt: io::Read {
     ///     Cursor::new(b"\xfd\xfd\xaa")
     ///         .read_compactsize().unwrap()
     /// );
-    /// assert_eq!(
+    /// ```
-    ///     0xbbaafd,
+    ///
-    ///     Cursor::new(b"\xfe\xfd\xaa\xbb\x00")
+    /// Sizes greater than the maximum network message length are invalid,
-    ///         .read_compactsize().unwrap()
+    /// they return a `Parse` error:
-    /// );
+    /// ```
-    /// assert_eq!(
+    /// # use zebra_chain::serialization::ReadZcashExt;
-    ///     0x22ccbbaafd,
+    /// # use std::io::Cursor;
-    ///     Cursor::new(b"\xff\xfd\xaa\xbb\xcc\x22\x00\x00\x00")
+    /// Cursor::new(b"\xfe\xfd\xaa\xbb\x00").read_compactsize().unwrap_err();
-    ///         .read_compactsize().unwrap()
+    /// Cursor::new(b"\xff\xfd\xaa\xbb\xcc\x22\x00\x00\x00").read_compactsize().unwrap_err();
-    /// );
     /// ```
     #[inline]
     fn read_compactsize(&mut self) -> Result<u64, SerializationError> {
         use SerializationError::Parse;
         let flag_byte = self.read_u8()?;
-        match flag_byte {
+        let size = match flag_byte {
             n @ 0x00..=0xfc => Ok(n as u64),
             0xfd => match self.read_u16::<LittleEndian>()? {
                 n @ 0x0000_00fd..=0x0000_ffff => Ok(n as u64),
@@ -70,7 +77,19 @@ pub trait ReadZcashExt: io::Read {
                 n @ 0x1_0000_0000..=0xffff_ffff_ffff_ffff => Ok(n),
                 _ => Err(Parse("non-canonical compactsize")),
             },
+        }?;
+
+        // # Security
+        // Defence-in-depth for memory DoS via preallocation.
+        if size
+            > MAX_PROTOCOL_MESSAGE_LEN
+                .try_into()
+                .expect("usize fits in u64")
+        {
+            Err(Parse("compactsize larger than protocol message limit"))?;
         }
+
+        Ok(size)
     }
 
     /// Read an IP address in Bitcoin format.

zebra-chain/src/serialization/write_zcash.rs

@@ -1,6 +1,10 @@
 use std::io;
-use std::net::{IpAddr, SocketAddr};
+use std::{
+    convert::TryInto,
+    net::{IpAddr, SocketAddr},
+};
 
+use super::MAX_PROTOCOL_MESSAGE_LEN;
 use byteorder::{BigEndian, LittleEndian, WriteBytesExt};
 
 /// Extends [`Write`] with methods for writing Zcash/Bitcoin types.
@@ -9,9 +13,16 @@ use byteorder::{BigEndian, LittleEndian, WriteBytesExt};
 pub trait WriteZcashExt: io::Write {
     /// Writes a `u64` using the Bitcoin `CompactSize` encoding.
     ///
+    /// # Panics
+    ///
+    /// Zebra panics on sizes greater than the protocol message length limit.
+    /// This is a defence-in-depth for memory preallocation attacks.
+    /// (These sizes should be impossible, because each array item takes at
+    /// least one byte.)
+    ///
     /// # Examples
     ///
-    /// ```rust
+    /// ```
     /// use zebra_chain::serialization::WriteZcashExt;
     ///
     /// let mut buf = Vec::new();
@@ -25,17 +36,28 @@ pub trait WriteZcashExt: io::Write {
     /// let mut buf = Vec::new();
     /// buf.write_compactsize(0xaafd).unwrap();
     /// assert_eq!(buf, b"\xfd\xfd\xaa");
+    /// ```
     ///
+    /// Sizes greater than the maximum network message length are invalid
+    /// and cause a panic:
+    /// ```should_panic
+    /// # use zebra_chain::serialization::WriteZcashExt;
     /// let mut buf = Vec::new();
-    /// buf.write_compactsize(0xbbaafd).unwrap();
+    /// buf.write_compactsize(0xbbaafd).unwrap_err();
-    /// assert_eq!(buf, b"\xfe\xfd\xaa\xbb\x00");
+    /// buf.write_compactsize(0x22ccbbaafd).unwrap_err();
-    ///
-    /// let mut buf = Vec::new();
-    /// buf.write_compactsize(0x22ccbbaafd).unwrap();
-    /// assert_eq!(buf, b"\xff\xfd\xaa\xbb\xcc\x22\x00\x00\x00");
     /// ```
     #[inline]
     fn write_compactsize(&mut self, n: u64) -> io::Result<()> {
+        // # Security
+        // Defence-in-depth for memory DoS via preallocation.
+        //
+        if n > MAX_PROTOCOL_MESSAGE_LEN
+            .try_into()
+            .expect("usize fits in u64")
+        {
+            panic!("compactsize larger than protocol message limit");
+        }
+
         match n {
             0x0000_0000..=0x0000_00fc => self.write_u8(n as u8),
             0x0000_00fd..=0x0000_ffff => {

zebra-chain/src/work/equihash.rs

@@ -108,6 +108,7 @@ impl ZcashDeserialize for Solution {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::block::MAX_BLOCK_BYTES;
 
     static EQUIHASH_SIZE_TESTS: &[u64] = &[
         0,
@@ -115,8 +116,8 @@ mod tests {
         (SOLUTION_SIZE - 1) as u64,
         SOLUTION_SIZE as u64,
         (SOLUTION_SIZE + 1) as u64,
-        u64::MAX - 1,
+        MAX_BLOCK_BYTES - 1,
-        u64::MAX,
+        MAX_BLOCK_BYTES,
     ];
 
     #[test]