Security: Avoid silently corrupting invalid times during serialization (#2149)

* Security: panic if an internally generated time is out of range

If Zebra has a bug where it generates blocks, transactions, or meta
addresses with bad times, panic. This avoids sending bad data onto the
network.

(Previously, Zebra would truncate some of these times, silently
corrupting the underlying data.)

Make it clear that deserialization of these objects is infalliable.

zebra-chain/src/block/serialize.rs

@@ -27,10 +27,12 @@ impl ZcashSerialize for Header {
         self.previous_block_hash.zcash_serialize(&mut writer)?;
         writer.write_all(&self.merkle_root.0[..])?;
         writer.write_all(&self.commitment_bytes[..])?;
-        // this is a truncating cast, rather than a saturating cast
+        writer.write_u32::<LittleEndian>(
-        // but u32 times are valid until 2106, and our block verification time
+            self.time
-        // checks should detect any truncation.
+                .timestamp()
-        writer.write_u32::<LittleEndian>(self.time.timestamp() as u32)?;
+                .try_into()
+                .expect("deserialized and generated timestamps are u32 values"),
+        )?;
         writer.write_u32::<LittleEndian>(self.difficulty_threshold.0)?;
         writer.write_all(&self.nonce[..])?;
         self.solution.zcash_serialize(&mut writer)?;
@@ -76,7 +78,7 @@ impl ZcashDeserialize for Header {
             merkle_root: merkle::Root(reader.read_32_bytes()?),
             commitment_bytes: reader.read_32_bytes()?,
             // This can't panic, because all u32 values are valid `Utc.timestamp`s
-            time: Utc.timestamp(reader.read_u32::<LittleEndian>()? as i64, 0),
+            time: Utc.timestamp(reader.read_u32::<LittleEndian>()?.into(), 0),
             difficulty_threshold: CompactDifficulty(reader.read_u32::<LittleEndian>()?),
             nonce: reader.read_32_bytes()?,
             solution: equihash::Solution::zcash_deserialize(reader)?,

zebra-chain/src/transaction/lock_time.rs

@@ -1,4 +1,4 @@
-use std::io;
+use std::{convert::TryInto, io};
 
 use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
 use chrono::{DateTime, TimeZone, Utc};
@@ -65,7 +65,8 @@ impl ZcashSerialize for LockTime {
         // we can always compute a hash of a transaction object.
         match self {
             LockTime::Height(block::Height(n)) => writer.write_u32::<LittleEndian>(*n)?,
-            LockTime::Time(t) => writer.write_u32::<LittleEndian>(t.timestamp() as u32)?,
+            LockTime::Time(t) => writer
+                .write_u32::<LittleEndian>(t.timestamp().try_into().expect("time is in range"))?,
         }
         Ok(())
     }
@@ -77,7 +78,8 @@ impl ZcashDeserialize for LockTime {
         if n <= block::Height::MAX.0 {
             Ok(LockTime::Height(block::Height(n)))
         } else {
-            Ok(LockTime::Time(Utc.timestamp(n as i64, 0)))
+            // This can't panic, because all u32 values are valid `Utc.timestamp`s
+            Ok(LockTime::Time(Utc.timestamp(n.into(), 0)))
         }
     }
 }

zebra-network/src/meta_addr.rs

@@ -2,6 +2,7 @@
 
 use std::{
     cmp::{Ord, Ordering},
+    convert::TryInto,
     io::{Read, Write},
     net::SocketAddr,
 };
@@ -309,7 +310,12 @@ impl PartialOrd for MetaAddr {
 
 impl ZcashSerialize for MetaAddr {
     fn zcash_serialize<W: Write>(&self, mut writer: W) -> Result<(), std::io::Error> {
-        writer.write_u32::<LittleEndian>(self.get_last_seen().timestamp() as u32)?;
+        writer.write_u32::<LittleEndian>(
+            self.get_last_seen()
+                .timestamp()
+                .try_into()
+                .expect("time is in range"),
+        )?;
         writer.write_u64::<LittleEndian>(self.services.bits())?;
         writer.write_socket_addr(self.addr)?;
         Ok(())
@@ -318,7 +324,8 @@ impl ZcashSerialize for MetaAddr {
 
 impl ZcashDeserialize for MetaAddr {
     fn zcash_deserialize<R: Read>(mut reader: R) -> Result<Self, SerializationError> {
-        let last_seen = Utc.timestamp(reader.read_u32::<LittleEndian>()? as i64, 0);
+        // This can't panic, because all u32 values are valid `Utc.timestamp`s
+        let last_seen = Utc.timestamp(reader.read_u32::<LittleEndian>()?.into(), 0);
         let services = PeerServices::from_bits_truncate(reader.read_u64::<LittleEndian>()?);
         let addr = reader.read_socket_addr()?;
 

zebra-network/src/meta_addr/arbitrary.rs

@@ -2,7 +2,8 @@ use proptest::{arbitrary::any, arbitrary::Arbitrary, prelude::*};
 
 use super::{MetaAddr, PeerAddrState, PeerServices};
 
-use std::{net::SocketAddr, time::SystemTime};
+use chrono::{TimeZone, Utc};
+use std::net::SocketAddr;
 
 impl Arbitrary for MetaAddr {
     type Parameters = ();
@@ -11,16 +12,15 @@ impl Arbitrary for MetaAddr {
         (
             any::<SocketAddr>(),
             any::<PeerServices>(),
-            any::<SystemTime>(),
+            any::<u32>(),
             any::<PeerAddrState>(),
         )
             .prop_map(
                 |(addr, services, last_seen, last_connection_state)| MetaAddr {
                     addr,
                     services,
-                    // TODO: implement constraints on last_seen as part of the
+                    // This can't panic, because all u32 values are valid `Utc.timestamp`s
-                    // last_connection_state refactor in #1849
+                    last_seen: Utc.timestamp(last_seen.into(), 0),
-                    last_seen: last_seen.into(),
                     last_connection_state,
                 },
             )