Merge pull request #3130 from wasmerio/fix-wasmer-cache-panic-2

Remove panics from Artifact::deserialize

CHANGELOG.md

@@ -15,6 +15,7 @@ Looking for changes that affect our C API? See the [C API Changelog](lib/c-api/C
 - #[3131](https://github.com/wasmerio/wasmer/pull/3131) Update migration docs for MemoryView changes
 
 ### Fixed
+- #[3130](https://github.com/wasmerio/wasmer/pull/3130) Remove panics from Artifact::deserialize
 
 ## 3.0.0-beta - 2022/08/08
 

lib/cli/src/c_gen/mod.rs

@@ -125,7 +125,7 @@ impl CType {
                 #[allow(clippy::borrowed_box)]
                 let ret: CType = return_value
                     .as_ref()
-                    .map(|i: &Box<CType>| (&**i).clone())
+                    .map(|i: &Box<CType>| (**i).clone())
                     .unwrap_or_default();
                 ret.generate_c(w);
                 w.push(' ');
@@ -183,7 +183,7 @@ impl CType {
                 #[allow(clippy::borrowed_box)]
                 let ret: CType = return_value
                     .as_ref()
-                    .map(|i: &Box<CType>| (&**i).clone())
+                    .map(|i: &Box<CType>| (**i).clone())
                     .unwrap_or_default();
                 ret.generate_c(w);
                 w.push(' ');

lib/compiler/src/artifact_builders/artifact_builder.rs

@@ -201,7 +201,7 @@ impl ArtifactCreate for ArtifactBuild {
     }
 
     fn data_initializers(&self) -> &[OwnedDataInitializer] {
-        &*self.serializable.data_initializers
+        &self.serializable.data_initializers
     }
 
     fn memory_styles(&self) -> &PrimaryMap<MemoryIndex, MemoryStyle> {

lib/compiler/src/engine/artifact.rs

@@ -116,9 +116,13 @@ impl Artifact {
                 "The provided bytes are not wasmer-universal".to_string(),
             ));
         }
-        let bytes = &bytes[ArtifactBuild::MAGIC_HEADER.len()..];
+
+        let bytes = Self::get_byte_slice(bytes, ArtifactBuild::MAGIC_HEADER.len(), bytes.len())?;
+
         let metadata_len = MetadataHeader::parse(bytes)?;
-        let metadata_slice: &[u8] = &bytes[MetadataHeader::LEN..][..metadata_len];
+        let metadata_slice = Self::get_byte_slice(bytes, MetadataHeader::LEN, bytes.len())?;
+        let metadata_slice = Self::get_byte_slice(metadata_slice, 0, metadata_len)?;
+
         let serializable = SerializableModule::deserialize(metadata_slice)?;
         let artifact = ArtifactBuild::from_serializable(serializable);
         let mut inner_engine = engine.inner_mut();
@@ -343,7 +347,7 @@ impl Artifact {
         // Get pointers to where metadata about local tables should live in VM memory.
 
         let (allocator, memory_definition_locations, table_definition_locations) =
-            InstanceAllocator::new(&*module);
+            InstanceAllocator::new(&module);
         let finished_memories = tunables
             .create_memories(
                 context,
@@ -400,7 +404,7 @@ impl Artifact {
             .iter()
             .map(|init| DataInitializer {
                 location: init.location.clone(),
-                data: &*init.data,
+                data: &init.data,
             })
             .collect::<Vec<_>>();
         handle
@@ -573,6 +577,19 @@ impl Artifact {
         ))
     }
 
+    fn get_byte_slice(input: &[u8], start: usize, end: usize) -> Result<&[u8], DeserializeError> {
+        if (start == end && input.len() > start)
+            || (start < end && input.len() > start && input.len() >= end)
+        {
+            Ok(&input[start..end])
+        } else {
+            Err(DeserializeError::InvalidByteLength {
+                expected: end - start,
+                got: input.len(),
+            })
+        }
+    }
+
     /// Deserialize a ArtifactBuild from an object file
     ///
     /// # Safety
@@ -583,15 +600,17 @@ impl Artifact {
         bytes: &[u8],
     ) -> Result<Self, DeserializeError> {
         let metadata_len = MetadataHeader::parse(bytes)?;
-
-        let metadata_slice = &bytes[MetadataHeader::LEN..][..metadata_len];
+        let metadata_slice = Self::get_byte_slice(bytes, MetadataHeader::LEN, bytes.len())?;
+        let metadata_slice = Self::get_byte_slice(metadata_slice, 0, metadata_len)?;
         let metadata: ModuleMetadata = ModuleMetadata::deserialize(metadata_slice)?;
 
         const WORD_SIZE: usize = mem::size_of::<usize>();
         let mut byte_buffer = [0u8; WORD_SIZE];
 
         let mut cur_offset = MetadataHeader::LEN + metadata_len;
-        byte_buffer[0..WORD_SIZE].clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);
+
+        let byte_buffer_slice = Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)?;
+        byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
         cur_offset += WORD_SIZE;
 
         let num_finished_functions = usize::from_ne_bytes(byte_buffer);
@@ -603,8 +622,9 @@ impl Artifact {
 
         // read finished functions in order now...
         for _i in 0..num_finished_functions {
-            byte_buffer[0..WORD_SIZE]
-                .clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);
+            let byte_buffer_slice =
+                Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)?;
+            byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
             let fp = FunctionBodyPtr(usize::from_ne_bytes(byte_buffer) as _);
             cur_offset += WORD_SIZE;
 
@@ -625,12 +645,15 @@ impl Artifact {
 
         // read trampolines in order
         let mut finished_function_call_trampolines = PrimaryMap::new();
-        byte_buffer[0..WORD_SIZE].clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);
+
+        let byte_buffer_slice = Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)?;
+        byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
         cur_offset += WORD_SIZE;
         let num_function_trampolines = usize::from_ne_bytes(byte_buffer);
         for _ in 0..num_function_trampolines {
-            byte_buffer[0..WORD_SIZE]
-                .clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);
+            let byte_buffer_slice =
+                Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)?;
+            byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
             cur_offset += WORD_SIZE;
             let trampoline_ptr_bytes = usize::from_ne_bytes(byte_buffer);
             let trampoline = mem::transmute::<usize, VMTrampoline>(trampoline_ptr_bytes);
@@ -640,12 +663,14 @@ impl Artifact {
 
         // read dynamic function trampolines in order now...
         let mut finished_dynamic_function_trampolines = PrimaryMap::new();
-        byte_buffer[0..WORD_SIZE].clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);
+        let byte_buffer_slice = Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)?;
+        byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
         cur_offset += WORD_SIZE;
         let num_dynamic_trampoline_functions = usize::from_ne_bytes(byte_buffer);
         for _i in 0..num_dynamic_trampoline_functions {
-            byte_buffer[0..WORD_SIZE]
-                .clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);
+            let byte_buffer_slice =
+                Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)?;
+            byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
             let fp = FunctionBodyPtr(usize::from_ne_bytes(byte_buffer) as _);
             cur_offset += WORD_SIZE;
 

lib/emscripten/src/lib.rs

@@ -105,7 +105,7 @@ impl EmEnv {
 
     /// Get a reference to the memory
     pub fn memory(&self, _mem_idx: u32) -> Memory {
-        (&*self.memory.read().unwrap()).as_ref().cloned().unwrap()
+        (*self.memory.read().unwrap()).as_ref().cloned().unwrap()
     }
 
     pub fn set_functions(&mut self, funcs: EmscriptenFunctions) {

lib/types/src/error.rs

@@ -35,10 +35,18 @@ pub enum DeserializeError {
     /// trying to allocate the required resources.
     #[error(transparent)]
     Compiler(#[from] CompileError),
+    /// Input artifact bytes have an invalid length
+    #[error("invalid input bytes: expected {expected} bytes, got {got}")]
+    InvalidByteLength {
+        /// How many bytes were expected
+        expected: usize,
+        /// How many bytes the artifact contained
+        got: usize,
+    },
 }
 
 /// Error type describing things that can go wrong when operating on Wasm Memories.
-#[derive(Error, Debug, Clone, PartialEq, Hash)]
+#[derive(Error, Debug, Clone, PartialEq, Eq, Hash)]
 pub enum MemoryError {
     /// Low level error with mmap.
     #[error("Error when allocating memory: {0}")]

lib/types/src/serialize.rs

@@ -144,7 +144,7 @@ impl SerializableModule {
 
     /// Returns data initializers to pass to `InstanceHandle::initialize`
     pub fn data_initializers(&self) -> &[OwnedDataInitializer] {
-        &*self.data_initializers
+        &self.data_initializers
     }
 
     /// Returns the memory styles associated with this `Artifact`.

lib/vm/src/instance/mod.rs

@@ -116,7 +116,7 @@ impl Instance {
     }
 
     pub(crate) fn module_ref(&self) -> &ModuleInfo {
-        &*self.module
+        &self.module
     }
 
     fn context(&self) -> &StoreObjects {
@@ -868,7 +868,7 @@ impl InstanceHandle {
                 let instance = instance_handle.instance_mut();
                 let vmctx_ptr = instance.vmctx_ptr();
                 (instance.funcrefs, instance.imported_funcrefs) = build_funcrefs(
-                    &*instance.module,
+                    &instance.module,
                     context,
                     &imports,
                     &instance.functions,

lib/vm/src/libcalls.rs

@@ -189,7 +189,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_memory32_grow(
 /// `vmctx` must be dereferenceable.
 #[no_mangle]
 pub unsafe extern "C" fn wasmer_vm_memory32_size(vmctx: *mut VMContext, memory_index: u32) -> u32 {
-    let instance = (&*vmctx).instance();
+    let instance = (*vmctx).instance();
     let memory_index = LocalMemoryIndex::from_u32(memory_index);
 
     instance.memory_size(memory_index).0
@@ -205,7 +205,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_memory32_size(
     vmctx: *mut VMContext,
     memory_index: u32,
 ) -> u32 {
-    let instance = (&*vmctx).instance();
+    let instance = (*vmctx).instance();
     let memory_index = MemoryIndex::from_u32(memory_index);
 
     instance.imported_memory_size(memory_index).0
@@ -303,7 +303,7 @@ pub unsafe extern "C" fn wasmer_vm_table_fill(
 /// `vmctx` must be dereferenceable.
 #[no_mangle]
 pub unsafe extern "C" fn wasmer_vm_table_size(vmctx: *mut VMContext, table_index: u32) -> u32 {
-    let instance = (&*vmctx).instance();
+    let instance = (*vmctx).instance();
     let table_index = LocalTableIndex::from_u32(table_index);
 
     instance.table_size(table_index)
@@ -319,7 +319,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_table_size(
     vmctx: *mut VMContext,
     table_index: u32,
 ) -> u32 {
-    let instance = (&*vmctx).instance();
+    let instance = (*vmctx).instance();
     let table_index = TableIndex::from_u32(table_index);
 
     instance.imported_table_size(table_index)
@@ -336,7 +336,7 @@ pub unsafe extern "C" fn wasmer_vm_table_get(
     table_index: u32,
     elem_index: u32,
 ) -> RawTableElement {
-    let instance = (&*vmctx).instance();
+    let instance = (*vmctx).instance();
     let table_index = LocalTableIndex::from_u32(table_index);
 
     // TODO: type checking, maybe have specialized accessors
@@ -495,7 +495,7 @@ pub unsafe extern "C" fn wasmer_vm_func_ref(
     vmctx: *mut VMContext,
     function_index: u32,
 ) -> VMFuncRef {
-    let instance = (&*vmctx).instance();
+    let instance = (*vmctx).instance();
     let function_index = FunctionIndex::from_u32(function_index);
 
     instance.func_ref(function_index).unwrap()
@@ -510,7 +510,7 @@ pub unsafe extern "C" fn wasmer_vm_func_ref(
 pub unsafe extern "C" fn wasmer_vm_elem_drop(vmctx: *mut VMContext, elem_index: u32) {
     on_host_stack(|| {
         let elem_index = ElemIndex::from_u32(elem_index);
-        let instance = (&*vmctx).instance();
+        let instance = (*vmctx).instance();
         instance.elem_drop(elem_index);
     })
 }
@@ -530,7 +530,7 @@ pub unsafe extern "C" fn wasmer_vm_memory32_copy(
 ) {
     let result = {
         let memory_index = LocalMemoryIndex::from_u32(memory_index);
-        let instance = (&*vmctx).instance();
+        let instance = (*vmctx).instance();
         instance.local_memory_copy(memory_index, dst, src, len)
     };
     if let Err(trap) = result {
@@ -553,7 +553,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_memory32_copy(
 ) {
     let result = {
         let memory_index = MemoryIndex::from_u32(memory_index);
-        let instance = (&*vmctx).instance();
+        let instance = (*vmctx).instance();
         instance.imported_memory_copy(memory_index, dst, src, len)
     };
     if let Err(trap) = result {
@@ -576,7 +576,7 @@ pub unsafe extern "C" fn wasmer_vm_memory32_fill(
 ) {
     let result = {
         let memory_index = LocalMemoryIndex::from_u32(memory_index);
-        let instance = (&*vmctx).instance();
+        let instance = (*vmctx).instance();
         instance.local_memory_fill(memory_index, dst, val, len)
     };
     if let Err(trap) = result {
@@ -599,7 +599,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_memory32_fill(
 ) {
     let result = {
         let memory_index = MemoryIndex::from_u32(memory_index);
-        let instance = (&*vmctx).instance();
+        let instance = (*vmctx).instance();
         instance.imported_memory_fill(memory_index, dst, val, len)
     };
     if let Err(trap) = result {
@@ -624,7 +624,7 @@ pub unsafe extern "C" fn wasmer_vm_memory32_init(
     let result = {
         let memory_index = MemoryIndex::from_u32(memory_index);
         let data_index = DataIndex::from_u32(data_index);
-        let instance = (&*vmctx).instance();
+        let instance = (*vmctx).instance();
         instance.memory_init(memory_index, data_index, dst, src, len)
     };
     if let Err(trap) = result {
@@ -641,7 +641,7 @@ pub unsafe extern "C" fn wasmer_vm_memory32_init(
 pub unsafe extern "C" fn wasmer_vm_data_drop(vmctx: *mut VMContext, data_index: u32) {
     on_host_stack(|| {
         let data_index = DataIndex::from_u32(data_index);
-        let instance = (&*vmctx).instance();
+        let instance = (*vmctx).instance();
         instance.data_drop(data_index)
     })
 }

lib/wasi/src/syscalls/mod.rs

@@ -307,7 +307,7 @@ pub fn args_get<M: MemorySize>(
     let env = ctx.data();
     let (memory, mut state) = env.get_memory_and_wasi_state(&ctx, 0);
 
-    let result = write_buffer_array(&memory, &*state.args, argv, argv_buf);
+    let result = write_buffer_array(&memory, &state.args, argv, argv_buf);
 
     debug!(
         "=> args:\n{}",
@@ -433,7 +433,7 @@ pub fn environ_get<M: MemorySize>(
     let (memory, mut state) = env.get_memory_and_wasi_state(&ctx, 0);
     trace!(" -> State envs: {:?}", state.envs);
 
-    write_buffer_array(&memory, &*state.envs, environ, environ_buf)
+    write_buffer_array(&memory, &state.envs, environ, environ_buf)
 }
 
 /// ### `environ_sizes_get()`
@@ -5555,7 +5555,7 @@ pub unsafe fn sock_send_file<M: MemorySize>(
             sock,
             __WASI_RIGHT_SOCK_SEND,
             |socket| {
-                let buf = (&buf[..]).to_vec();
+                let buf = (buf[..]).to_vec();
                 socket.send_bytes::<M>(Bytes::from(buf))
             }
         ));

tests/compilers/serialize.rs

@@ -1,6 +1,13 @@
 use anyhow::Result;
 use wasmer::*;
 
+#[test]
+fn sanity_test_artifact_deserialize() {
+    let engine = Engine::headless();
+    let result = unsafe { Artifact::deserialize(&engine, &[]) };
+    assert!(result.is_err());
+}
+
 #[compiler_test(serialize)]
 fn test_serialize(config: crate::Config) -> Result<()> {
     let mut store = config.store();