Don't allow deserializing names which shouldn't exist

This addresses the potential security issue discussed in the last patch
by implementing custom versions of `deserialize` that check for objects
corresponding to each name.

src/elements/index_map.rs

@@ -128,6 +128,49 @@ impl<T> IndexMap<T> {
         // Note that this does the right thing because we use `&self`.
         self.into_iter()
     }
+
+    /// Custom deserialization routine.
+    ///
+    /// We will allocate an underlying array no larger than `max_entry_space` to
+    /// hold the data, so the maximum index must be less than `max_entry_space`.
+    /// This prevents mallicious *.wasm files from having a single entry with
+    /// the index `u32::MAX`, which would consume far too much memory.
+    ///
+    /// The `deserialize_value` function will be passed the index of the value
+    /// being deserialized, and must deserialize the value.
+    pub fn deserialize_with<R, F>(
+        max_entry_space: usize,
+        deserialize_value: &F,
+        rdr: &mut R,
+    ) -> Result<IndexMap<T>, Error>
+    where
+        R: Read,
+        F: Fn(u32, &mut R) -> Result<T, Error>,
+    {
+        let len: u32 = VarUint32::deserialize(rdr)?.into();
+        let mut map = IndexMap::with_capacity(len as usize);
+        let mut prev_idx = None;
+        for _ in 0..len {
+            let idx: u32 = VarUint32::deserialize(rdr)?.into();
+            if idx as usize >= max_entry_space {
+                return Err(Error::Other("index is larger than expected"));
+            }
+            match prev_idx {
+                Some(prev) if prev >= idx => {
+                    // Supposedly these names must be "sorted by index", so
+                    // let's try enforcing that and seeing what happens.
+                    return Err(Error::Other("indices are out of order"));
+                }
+                _ => {
+                    prev_idx = Some(idx);
+                }
+            }
+            let val = deserialize_value(idx, rdr)?;
+            map.insert(idx, val);
+        }
+        Ok(map)
+    }
+
 }
 
 impl<T: Clone> Clone for IndexMap<T> {
@@ -286,34 +329,22 @@ where
     }
 }
 
-impl<T: Deserialize> Deserialize for IndexMap<T>
+impl<T: Deserialize> IndexMap<T>
 where
     T: Deserialize,
     Error: From<<T as Deserialize>::Error>,
 {
-    type Error = Error;
+    /// Deserialize a map containing simple values that support `Deserialize`.
-
+    /// We will allocate an underlying array no larger than `max_entry_space` to
-    fn deserialize<R: Read>(rdr: &mut R) -> Result<Self, Self::Error> {
+    /// hold the data, so the maximum index must be less than `max_entry_space`.
-        // SECURITY: Implemented wasted space checking.
+    pub fn deserialize<R: Read>(
-        let len: u32 = VarUint32::deserialize(rdr)?.into();
+        max_entry_space: usize,
-        let mut map = IndexMap::with_capacity(len as usize);
+        rdr: &mut R,
-        let mut prev_idx = None;
+    ) -> Result<Self, Error> {
-        for _ in 0..len {
+        let deserialize_value: fn(u32, &mut R) -> Result<T, Error> = |_idx, rdr| {
-            let idx: u32 = VarUint32::deserialize(rdr)?.into();
+            T::deserialize(rdr).map_err(Error::from)
-            match prev_idx {
+        };
-                Some(prev) if prev >= idx => {
+        Self::deserialize_with(max_entry_space, &deserialize_value, rdr)
-                    // Supposedly these names must be "sorted by index", so
-                    // let's try enforcing that and seeing what happens.
-                    return Err(Error::Other("indices are out of order"));
-                }
-                _ => {
-                    prev_idx = Some(idx);
-                }
-            }
-            let val = T::deserialize(rdr)?;
-            map.insert(idx, val);
-        }
-        Ok(map)
     }
 }
 
@@ -513,7 +544,7 @@ mod tests {
             .expect("serialize failed");
 
         let mut input = io::Cursor::new(&output);
-        let deserialized = IndexMap::deserialize(&mut input).expect("deserialize failed");
+        let deserialized = IndexMap::deserialize(2, &mut input).expect("deserialize failed");
 
         assert_eq!(deserialized, map);
     }
@@ -527,7 +558,7 @@ mod tests {
         "val 0".to_string().serialize(&mut valid).unwrap();
         VarUint32::from(1u32).serialize(&mut valid).unwrap();
         "val 1".to_string().serialize(&mut valid).unwrap();
-        let map = IndexMap::<String>::deserialize(&mut io::Cursor::new(valid))
+        let map = IndexMap::<String>::deserialize(2, &mut io::Cursor::new(valid))
             .expect("unexpected error deserializing");
         assert_eq!(map.len(), 2);
 
@@ -538,7 +569,18 @@ mod tests {
         "val 1".to_string().serialize(&mut invalid).unwrap();
         VarUint32::from(0u32).serialize(&mut invalid).unwrap();
         "val 0".to_string().serialize(&mut invalid).unwrap();
-        let res = IndexMap::<String>::deserialize(&mut io::Cursor::new(invalid));
+        let res = IndexMap::<String>::deserialize(2, &mut io::Cursor::new(invalid));
+        assert!(res.is_err());
+    }
+
+    #[test]
+    fn deserialize_enforces_max_idx() {
+        // Build an example with an out-of-bounds index by hand.
+        let mut invalid = vec![];
+        VarUint32::from(1u32).serialize(&mut invalid).unwrap();
+        VarUint32::from(5u32).serialize(&mut invalid).unwrap();
+        "val 5".to_string().serialize(&mut invalid).unwrap();
+        let res = IndexMap::<String>::deserialize(1, &mut io::Cursor::new(invalid));
         assert!(res.is_err());
     }
 }

src/elements/name_section.rs

@@ -1,6 +1,6 @@
 use std::io::{Read, Write};
 
-use super::{Deserialize, Error, Serialize, VarUint32, VarUint7};
+use super::{Deserialize, Error, Module, Serialize, VarUint32, VarUint7};
 use super::index_map::IndexMap;
 
 const NAME_TYPE_MODULE: u8 = 0;
@@ -28,6 +28,31 @@ pub enum NameSection {
     },
 }
 
+impl NameSection {
+    /// Deserialize a name section.
+    pub fn deserialize<R: Read>(
+        module: &Module,
+        rdr: &mut R,
+    ) -> Result<NameSection, Error> {
+        let name_type: u8 = VarUint7::deserialize(rdr)?.into();
+        let name_payload_len: u32 = VarUint32::deserialize(rdr)?.into();
+        let name_section = match name_type {
+            NAME_TYPE_MODULE => NameSection::Module(ModuleNameSection::deserialize(rdr)?),
+            NAME_TYPE_FUNCTION => NameSection::Function(FunctionNameSection::deserialize(module, rdr)?),
+            NAME_TYPE_LOCAL => NameSection::Local(LocalNameSection::deserialize(module, rdr)?),
+            _ => {
+                let mut name_payload = vec![0u8; name_payload_len as usize];
+                rdr.read_exact(&mut name_payload)?;
+                NameSection::Unparsed {
+                    name_type,
+                    name_payload,
+                }
+            }
+        };
+        Ok(name_section)
+    }
+}
+
 impl Serialize for NameSection {
     type Error = Error;
 
@@ -60,29 +85,6 @@ impl Serialize for NameSection {
     }
 }
 
-impl Deserialize for NameSection {
-    type Error = Error;
-
-    fn deserialize<R: Read>(rdr: &mut R) -> Result<NameSection, Error> {
-        let name_type: u8 = VarUint7::deserialize(rdr)?.into();
-        let name_payload_len: u32 = VarUint32::deserialize(rdr)?.into();
-        let name_section = match name_type {
-            NAME_TYPE_MODULE => NameSection::Module(ModuleNameSection::deserialize(rdr)?),
-            NAME_TYPE_FUNCTION => NameSection::Function(FunctionNameSection::deserialize(rdr)?),
-            NAME_TYPE_LOCAL => NameSection::Local(LocalNameSection::deserialize(rdr)?),
-            _ => {
-                let mut name_payload = vec![0u8; name_payload_len as usize];
-                rdr.read_exact(&mut name_payload)?;
-                NameSection::Unparsed {
-                    name_type,
-                    name_payload,
-                }
-            }
-        };
-        Ok(name_section)
-    }
-}
-
 /// The name of this module.
 #[derive(Clone, Debug, PartialEq)]
 pub struct ModuleNameSection {
@@ -139,6 +141,19 @@ impl FunctionNameSection {
     pub fn names_mut(&mut self) -> &mut NameMap {
         &mut self.names
     }
+
+    /// Deserialize names, making sure that all names correspond to functions.
+    pub fn deserialize<R: Read>(
+        module: &Module,
+        rdr: &mut R,
+    ) -> Result<FunctionNameSection, Error> {
+        let funcs = module.function_section().ok_or_else(|| {
+            Error::Other("cannot deserialize names without a function section")
+        })?;
+        let max_entry_space = funcs.entries().len();
+        let names = IndexMap::deserialize(max_entry_space, rdr)?;
+        Ok(FunctionNameSection { names })
+    }
 }
 
 impl Serialize for FunctionNameSection {
@@ -149,15 +164,6 @@ impl Serialize for FunctionNameSection {
     }
 }
 
-impl Deserialize for FunctionNameSection {
-    type Error = Error;
-
-    fn deserialize<R: Read>(rdr: &mut R) -> Result<FunctionNameSection, Error> {
-        let names = IndexMap::deserialize(rdr)?;
-        Ok(FunctionNameSection { names })
-    }
-}
-
 /// The names of the local variables in this module's functions.
 #[derive(Clone, Debug, Default, PartialEq)]
 pub struct LocalNameSection {
@@ -175,7 +181,33 @@ impl LocalNameSection {
     pub fn local_names_mut(&mut self) -> &mut IndexMap<NameMap> {
         &mut self.local_names
     }
-}
+
+    /// Deserialize names, making sure that all names correspond to local
+    /// variables.
+    pub fn deserialize<R: Read>(
+        module: &Module,
+        rdr: &mut R,
+    ) -> Result<LocalNameSection, Error> {
+        let funcs = module.function_section().ok_or_else(|| {
+            Error::Other("cannot deserialize local names without a function section")
+        })?;
+        let max_entry_space = funcs.entries().len();
+
+        let code = module.code_section().ok_or_else(|| {
+            Error::Other("cannot deserialize local names without a code section")
+        })?;
+        let deserialize_locals = |idx: u32, rdr: &mut R| {
+            let max_local_entry_space = code.bodies()[idx as usize].locals().len();
+            IndexMap::deserialize(max_local_entry_space, rdr)
+        };
+
+        let local_names = IndexMap::deserialize_with(
+            max_entry_space,
+            &deserialize_locals,
+            rdr,
+        )?;
+        Ok(LocalNameSection { local_names })
+    }}
 
 impl Serialize for LocalNameSection {
     type Error = Error;
@@ -185,15 +217,6 @@ impl Serialize for LocalNameSection {
     }
 }
 
-impl Deserialize for LocalNameSection {
-    type Error = Error;
-
-    fn deserialize<R: Read>(rdr: &mut R) -> Result<LocalNameSection, Error> {
-        let local_names = IndexMap::deserialize(rdr)?;
-        Ok(LocalNameSection { local_names })
-    }
-}
-
 /// A map from indices to names.
 pub type NameMap = IndexMap<String>;
 
@@ -205,48 +228,45 @@ mod tests {
 
     // A helper funtion for the tests. Serialize a section, deserialize it,
     // and make sure it matches the original.
-    fn serialize_and_deserialize(original: NameSection) {
+    fn serialize_test(original: NameSection) -> Vec<u8> {
         let mut buffer = vec![];
         original
-            .clone()
             .serialize(&mut buffer)
             .expect("serialize error");
-        let mut input = Cursor::new(buffer);
+        buffer
-        let deserialized = NameSection::deserialize(&mut input).expect("deserialize error");
-        assert_eq!(deserialized, original);
     }
 
     #[test]
-    fn serialize_and_deserialize_module_name() {
+    fn serialize_module_name() {
-        let sect = ModuleNameSection::new("my_mod");
+        let original = NameSection::Module(ModuleNameSection::new("my_mod"));
-        serialize_and_deserialize(NameSection::Module(sect));
+        serialize_test(original.clone());
     }
 
     #[test]
-    fn serialize_and_deserialize_function_names() {
+    fn serialize_function_names() {
         let mut sect = FunctionNameSection::default();
         sect.names_mut().insert(0, "hello_world".to_string());
-        serialize_and_deserialize(NameSection::Function(sect));
+        serialize_test(NameSection::Function(sect));
     }
 
     #[test]
-    fn serialize_and_deserialize_local_names() {
+    fn serialize_local_names() {
         let mut sect = LocalNameSection::default();
         let mut locals = NameMap::default();
         locals.insert(0, "msg".to_string());
         sect.local_names_mut().insert(0, locals);
-        serialize_and_deserialize(NameSection::Local(sect));
+        serialize_test(NameSection::Local(sect));
     }
 
     #[test]
     fn serialize_and_deserialize_unparsed() {
-        let sect = NameSection::Unparsed {
+        let original = NameSection::Unparsed {
             // A made-up name section type which is unlikely to be allocated
             // soon, in order to allow us to test `Unparsed`.
             name_type: 120,
             name_payload: vec![0u8, 1, 2],
         };
-        serialize_and_deserialize(sect);
+        serialize_test(original.clone());
     }
 
     #[test]
@@ -261,7 +281,7 @@ mod tests {
             match *s {
                 Section::Custom(ref cs) if cs.name() == "name" => {
                     let mut cursor = Cursor::new(cs.payload());
-                    let section = NameSection::deserialize(&mut cursor)
+                    let section = NameSection::deserialize(&module, &mut cursor)
                         .expect("could not parse name section");
                     if let NameSection::Function(function_names) = section {
                         function_names_opt = Some(function_names);