Implement limits for Vec, String.

Enforces a maximum byte limit both during serialization and
deserialization. Currently we use hardcoded values, but in the
future we need to make this configurable at run time.

Signed-off-by: Andrei Sandu <[email protected]>

Author: sandreim

coverage_config_aarch64.json

@@ -1 +1 @@
-{"coverage_score": 92.7, "exclude_path": "", "crate_features": ""}
+{"coverage_score": 92.2, "exclude_path": "", "crate_features": ""}

coverage_config_x86_64.json

@@ -1 +1 @@
-{"coverage_score": 93.2, "exclude_path": "", "crate_features": ""}
+{"coverage_score": 92.8, "exclude_path": "", "crate_features": ""}

src/lib.rs

@@ -21,7 +21,7 @@ extern crate vm_memory;
 extern crate vmm_sys_util;
 
 pub mod crc;
-mod primitives;
+pub mod primitives;
 pub mod version_map;
 
 use std::any::TypeId;
@@ -40,16 +40,33 @@ pub enum VersionizeError {
     Deserialize(String),
     /// A user generated semantic error.
     Semantic(String),
+    /// String length exceeded.
+    StringLength(usize),
+    /// Vector length exceeded.
+    VecLength(usize),
 }
 
 impl std::fmt::Display for VersionizeError {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::result::Result<(), std::fmt::Error> {
         use VersionizeError::*;
+
         match self {
             Io(e) => write!(f, "An IO error occured: {}", e),
             Serialize(e) => write!(f, "A serialization error occured: {}", e),
             Deserialize(e) => write!(f, "A deserialization error occured: {}", e),
             Semantic(e) => write!(f, "A user generated semantic error occured: {}", e),
+            StringLength(bad_len) => write!(
+                f,
+                "String length exceeded {} > {} bytes",
+                bad_len,
+                primitives::MAX_STRING_LEN
+            ),
+            VecLength(bad_len) => write!(
+                f,
+                "Vec length exceeded {} > {} bytes",
+                bad_len,
+                primitives::MAX_VEC_LEN
+            ),
         }
     }
 }

src/primitives.rs

@@ -1,10 +1,16 @@
 // Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
+//! Serialization support for primitive data types.
 #![allow(clippy::float_cmp)]
 
 use self::super::{VersionMap, Versionize, VersionizeError, VersionizeResult};
 use vmm_sys_util::fam::{FamStruct, FamStructWrapper};
 
+/// Maximum string len in bytes (16KB).
+pub const MAX_STRING_LEN: usize = 16384;
+/// Maximum vec len in bytes (10MB).
+pub const MAX_VEC_LEN: usize = 10_485_760;
+
 /// Implements the Versionize trait for primitive types that also implement
 /// serde's Serialize/Deserialize: use serde_bincode as a backend for
 /// serialization.
@@ -60,7 +66,53 @@ impl_versionize!(u64);
 impl_versionize!(f32);
 impl_versionize!(f64);
 impl_versionize!(char);
-impl_versionize!(String);
+
+impl Versionize for String {
+    #[inline]
+    fn serialize<W: std::io::Write>(
+        &self,
+        writer: &mut W,
+        version_map: &VersionMap,
+        app_version: u16,
+    ) -> VersionizeResult<()> {
+        // It is better to fail early at serialization time.
+        if self.len() > MAX_STRING_LEN {
+            return Err(VersionizeError::StringLength(self.len()));
+        }
+
+        self.len().serialize(writer, version_map, app_version)?;
+        writer
+            .write_all(self.as_bytes())
+            .map_err(|e| VersionizeError::Io(e.raw_os_error().unwrap_or(0)))?;
+        Ok(())
+    }
+
+    #[inline]
+    fn deserialize<R: std::io::Read>(
+        mut reader: &mut R,
+        version_map: &VersionMap,
+        app_version: u16,
+    ) -> VersionizeResult<Self> {
+        let len = usize::deserialize(&mut reader, version_map, app_version)?;
+        // Even if we fail in serialize, we still need to enforce this on the hot path
+        // in case the len is corrupted.
+        if len > MAX_STRING_LEN {
+            return Err(VersionizeError::StringLength(len));
+        }
+
+        let mut v = vec![0u8; len];
+        reader
+            .read_exact(v.as_mut_slice())
+            .map_err(|e| VersionizeError::Io(e.raw_os_error().unwrap_or(0)))?;
+        Ok(String::from_utf8(v)
+            .map_err(|err| VersionizeError::Deserialize(format!("Utf8 error: {:?}", err)))?)
+    }
+
+    // Not used yet.
+    fn version() -> u16 {
+        1
+    }
+}
 
 macro_rules! impl_versionize_array_with_size {
     ($ty:literal) => {
@@ -239,15 +291,17 @@ where
         version_map: &VersionMap,
         app_version: u16,
     ) -> VersionizeResult<()> {
+        let bytes_len = self.len() * std::mem::size_of::<T>();
+        if bytes_len > MAX_VEC_LEN {
+            return Err(VersionizeError::VecLength(bytes_len));
+        }
         // Serialize in the same fashion as bincode:
         // Write len.
         bincode::serialize_into(&mut writer, &self.len())
             .map_err(|ref err| VersionizeError::Serialize(format!("{:?}", err)))?;
-        // Walk the vec and write each elemenet.
+        // Walk the vec and write each element.
         for element in self {
-            element
-                .serialize(writer, version_map, app_version)
-                .map_err(|ref err| VersionizeError::Serialize(format!("{:?}", err)))?;
+            element.serialize(writer, version_map, app_version)?;
         }
         Ok(())
     }
@@ -259,8 +313,14 @@ where
         app_version: u16,
     ) -> VersionizeResult<Self> {
         let mut v = Vec::new();
-        let len: u64 = bincode::deserialize_from(&mut reader)
+        let len: usize = bincode::deserialize_from(&mut reader)
             .map_err(|ref err| VersionizeError::Deserialize(format!("{:?}", err)))?;
+
+        let bytes_len = len * std::mem::size_of::<T>();
+        if bytes_len > MAX_VEC_LEN {
+            return Err(VersionizeError::VecLength(bytes_len));
+        }
+
         for _ in 0..len {
             let element: T = T::deserialize(reader, version_map, app_version)
                 .map_err(|ref err| VersionizeError::Deserialize(format!("{:?}", err)))?;
@@ -935,4 +995,33 @@ mod tests {
         );
         assert_eq!(original_values, restored_values);
     }
+
+    #[test]
+    fn test_vec_limit() {
+        // We need extra 8 bytes for vector len.
+        let mut snapshot_mem = vec![0u8; MAX_VEC_LEN + 8];
+        let err = vec![123u8; MAX_VEC_LEN + 1]
+            .serialize(&mut snapshot_mem.as_mut_slice(), &VersionMap::new(), 1)
+            .unwrap_err();
+        assert_eq!(err, VersionizeError::VecLength(MAX_VEC_LEN + 1));
+        assert_eq!(
+            format!("{}", err),
+            "Vec length exceeded 10485761 > 10485760 bytes"
+        );
+    }
+
+    #[test]
+    fn test_string_limit() {
+        // We need extra 8 bytes for string len.
+        let mut snapshot_mem = vec![0u8; MAX_STRING_LEN + 8];
+        let err = String::from_utf8(vec![123u8; MAX_STRING_LEN + 1])
+            .unwrap()
+            .serialize(&mut snapshot_mem.as_mut_slice(), &VersionMap::new(), 1)
+            .unwrap_err();
+        assert_eq!(err, VersionizeError::StringLength(MAX_STRING_LEN + 1));
+        assert_eq!(
+            format!("{}", err),
+            "String length exceeded 16385 > 16384 bytes"
+        );
+    }
 }