Rework Module deserialize methods

Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727

Author: theduke

lib/api/src/engine.rs

@@ -135,24 +135,57 @@ impl Engine {
         ))
     }
 
-    #[deprecated(since = "3.2.0")]
     #[cfg(all(feature = "sys", not(target_arch = "wasm32")))]
-    /// Deserializes a WebAssembly module
+    /// Deserializes a WebAssembly module which was previously serialized with
+    /// `Module::serialize`.
+    ///
+    /// NOTE: you should almost always prefer [`Self::deserialize`].
     ///
     /// # Safety
+    /// See [`Artifact::deserialize_unchecked`].
+    pub unsafe fn deserialize_unchecked(
+        &self,
+        bytes: &[u8],
+    ) -> Result<Arc<Artifact>, DeserializeError> {
+        Ok(Arc::new(Artifact::deserialize_unchecked(&self.0, bytes)?))
+    }
+
+    #[cfg(all(feature = "sys", not(target_arch = "wasm32")))]
+    /// Deserializes a WebAssembly module which was previously serialized with
+    /// `Module::serialize`.
     ///
-    /// The serialized content must represent a serialized WebAssembly module.
+    /// # Safety
+    /// See [`Artifact::deserialize`].
     pub unsafe fn deserialize(&self, bytes: &[u8]) -> Result<Arc<Artifact>, DeserializeError> {
         Ok(Arc::new(Artifact::deserialize(&self.0, bytes)?))
     }
 
-    #[deprecated(since = "3.2.0")]
     #[cfg(all(feature = "sys", not(target_arch = "wasm32")))]
-    /// Deserializes a WebAssembly module from a path
+    /// Load a serialized WebAssembly module from a file and deserialize it.
+    ///
+    /// NOTE: you should almost always prefer [`Self::deserialize_from_file`].
     ///
     /// # Safety
+    /// See [`Artifact::deserialize_unchecked`].
+    pub unsafe fn deserialize_from_file_unchecked(
+        &self,
+        file_ref: &Path,
+    ) -> Result<Arc<Artifact>, DeserializeError> {
+        let mut file = std::fs::File::open(file_ref)?;
+        let mut buffer = Vec::new();
+        // read the whole file
+        file.read_to_end(&mut buffer)?;
+        Ok(Arc::new(Artifact::deserialize_unchecked(
+            &self.0,
+            buffer.as_slice(),
+        )?))
+    }
+
+    #[cfg(all(feature = "sys", not(target_arch = "wasm32")))]
+    /// Load a serialized WebAssembly module from a file and deserialize it.
     ///
-    /// The file's content must represent a serialized WebAssembly module.
+    /// # Safety
+    /// See [`Artifact::deserialize`].
     pub unsafe fn deserialize_from_file(
         &self,
         file_ref: &Path,

lib/api/src/js/module.rs

@@ -227,7 +227,7 @@ impl Module {
         ));
     }
 
-    pub unsafe fn deserialize(
+    pub unsafe fn deserialize_unchecked(
         _engine: &impl AsEngineRef,
         _bytes: impl IntoBytes,
     ) -> Result<Self, DeserializeError> {
@@ -239,7 +239,7 @@ impl Module {
         return Err(DeserializeError::Generic("You need to enable the `js-serializable-module` feature flag to deserialize a `Module`".to_string()));
     }
 
-    pub fn deserialize_checked(
+    pub unsafe fn deserialize(
         _engine: &impl AsEngineRef,
         _bytes: impl IntoBytes,
     ) -> Result<Self, DeserializeError> {
@@ -251,20 +251,20 @@ impl Module {
         return Err(DeserializeError::Generic("You need to enable the `js-serializable-module` feature flag to deserialize a `Module`".to_string()));
     }
 
-    pub unsafe fn deserialize_from_file(
+    pub unsafe fn deserialize_from_file_unchecked(
         engine: &impl AsEngineRef,
         path: impl AsRef<Path>,
     ) -> Result<Self, DeserializeError> {
         let bytes = std::fs::read(path.as_ref())?;
         Self::deserialize(engine, bytes)
     }
 
-    pub fn deserialize_from_file_checked(
+    pub unsafe fn deserialize_from_file(
         engine: &impl AsEngineRef,
         path: impl AsRef<Path>,
     ) -> Result<Self, DeserializeError> {
         let bytes = std::fs::read(path.as_ref())?;
-        Self::deserialize_checked(engine, bytes)
+        Self::deserialize(engine, bytes)
     }
 
     pub fn set_name(&mut self, name: &str) -> bool {

lib/api/src/jsc/module.rs

@@ -183,35 +183,35 @@ impl Module {
         ));
     }
 
-    pub unsafe fn deserialize(
+    pub unsafe fn deserialize_unchecked(
         _engine: &impl AsEngineRef,
         _bytes: impl IntoBytes,
     ) -> Result<Self, DeserializeError> {
         return Self::from_binary(_engine, &_bytes.into_bytes())
             .map_err(|e| DeserializeError::Compiler(e));
     }
 
-    pub fn deserialize_checked(
+    pub unsafe fn deserialize(
         _engine: &impl AsEngineRef,
         _bytes: impl IntoBytes,
     ) -> Result<Self, DeserializeError> {
         unimplemented!();
     }
 
-    pub unsafe fn deserialize_from_file(
+    pub unsafe fn deserialize_from_file_unchecked(
         engine: &impl AsEngineRef,
         path: impl AsRef<Path>,
     ) -> Result<Self, DeserializeError> {
         let bytes = std::fs::read(path.as_ref())?;
-        Self::deserialize(engine, bytes)
+        Self::deserialize_unchecked(engine, bytes)
     }
 
-    pub fn deserialize_from_file_checked(
+    pub unsafe fn deserialize_from_file(
         engine: &impl AsEngineRef,
         path: impl AsRef<Path>,
     ) -> Result<Self, DeserializeError> {
         let bytes = std::fs::read(path.as_ref())?;
-        Self::deserialize_checked(engine, bytes)
+        Self::deserialize(engine, bytes)
     }
 
     pub fn set_name(&mut self, name: &str) -> bool {

lib/api/src/module.rs

@@ -219,9 +219,9 @@ impl Module {
         Ok(())
     }
 
-    /// Deserializes a serialized Module binary into a `Module`.
+    /// Deserializes a serialized module binary into a `Module`.
     ///
-    /// Note: You should usually prefer the safe [`Module::deserialize_checked`].
+    /// Note: You should usually prefer the safer [`Module::deserialize`].
     ///
     /// # Important
     ///
@@ -250,11 +250,13 @@ impl Module {
     /// # Ok(())
     /// # }
     /// ```
-    pub unsafe fn deserialize(
+    pub unsafe fn deserialize_unchecked(
         engine: &impl AsEngineRef,
         bytes: impl IntoBytes,
     ) -> Result<Self, DeserializeError> {
-        Ok(Self(module_imp::Module::deserialize(engine, bytes)?))
+        Ok(Self(module_imp::Module::deserialize_unchecked(
+            engine, bytes,
+        )?))
     }
 
     /// Deserializes a serialized Module binary into a `Module`.
@@ -272,17 +274,21 @@ impl Module {
     /// # use wasmer::*;
     /// # fn main() -> anyhow::Result<()> {
     /// # let mut store = Store::default();
-    /// let module = Module::deserialize_checked(&store, serialized_data)?;
+    /// let module = Module::deserialize(&store, serialized_data)?;
     /// # Ok(())
     /// # }
     /// ```
-    pub fn deserialize_checked(
+    ///
+    /// # Safety
+    /// This function is inherently **unsafe**, because it loads executable code
+    /// into memory.
+    /// The loaded bytes must be trusted to contain a valid artifact previously
+    /// built with [`Self::serialize`].
+    pub unsafe fn deserialize(
         engine: &impl AsEngineRef,
         bytes: impl IntoBytes,
     ) -> Result<Self, DeserializeError> {
-        Ok(Self(module_imp::Module::deserialize_checked(
-            engine, bytes,
-        )?))
+        Ok(Self(module_imp::Module::deserialize(engine, bytes)?))
     }
 
     /// Deserializes a a serialized Module located in a `Path` into a `Module`.
@@ -298,37 +304,43 @@ impl Module {
     /// # Ok(())
     /// # }
     /// ```
-    pub fn deserialize_from_file_checked(
+    ///
+    /// # Safety
+    ///
+    /// See [`Self::deserialize`].
+    pub unsafe fn deserialize_from_file(
         engine: &impl AsEngineRef,
         path: impl AsRef<Path>,
     ) -> Result<Self, DeserializeError> {
-        Ok(Self(module_imp::Module::deserialize_from_file_checked(
+        Ok(Self(module_imp::Module::deserialize_from_file(
             engine, path,
         )?))
     }
 
     /// Deserializes a a serialized Module located in a `Path` into a `Module`.
     /// > Note: the module has to be serialized before with the `serialize` method.
     ///
+    /// You should usually prefer the safer [`Module::deserialize_from_file`].
+    ///
     /// # Safety
     ///
-    /// Please check [`Module::deserialize`].
+    /// Please check [`Module::deserialize_unchecked`].
     ///
     /// # Usage
     ///
     /// ```ignore
     /// # use wasmer::*;
     /// # let mut store = Store::default();
     /// # fn main() -> anyhow::Result<()> {
-    /// let module = Module::deserialize_from_file(&store, path)?;
+    /// let module = Module::deserialize_from_file_unchecked(&store, path)?;
     /// # Ok(())
     /// # }
     /// ```
-    pub unsafe fn deserialize_from_file(
+    pub unsafe fn deserialize_from_file_unchecked(
         engine: &impl AsEngineRef,
         path: impl AsRef<Path>,
     ) -> Result<Self, DeserializeError> {
-        Ok(Self(module_imp::Module::deserialize_from_file(
+        Ok(Self(module_imp::Module::deserialize_from_file_unchecked(
             engine, path,
         )?))
     }

lib/api/src/sys/module.rs

@@ -69,7 +69,7 @@ impl Module {
         self.artifact.serialize().map(|bytes| bytes.into())
     }
 
-    pub unsafe fn deserialize(
+    pub unsafe fn deserialize_unchecked(
         engine: &impl AsEngineRef,
         bytes: impl IntoBytes,
     ) -> Result<Self, DeserializeError> {
@@ -78,20 +78,16 @@ impl Module {
         Ok(Self::from_artifact(artifact))
     }
 
-    pub fn deserialize_checked(
+    pub unsafe fn deserialize(
         engine: &impl AsEngineRef,
         bytes: impl IntoBytes,
     ) -> Result<Self, DeserializeError> {
         let bytes = bytes.into_bytes();
-        let artifact = engine
-            .as_engine_ref()
-            .engine()
-            .0
-            .deserialize_checked(&bytes)?;
+        let artifact = engine.as_engine_ref().engine().0.deserialize(&bytes)?;
         Ok(Self::from_artifact(artifact))
     }
 
-    pub unsafe fn deserialize_from_file(
+    pub unsafe fn deserialize_from_file_unchecked(
         engine: &impl AsEngineRef,
         path: impl AsRef<Path>,
     ) -> Result<Self, DeserializeError> {
@@ -103,15 +99,15 @@ impl Module {
         Ok(Self::from_artifact(artifact))
     }
 
-    pub fn deserialize_from_file_checked(
+    pub unsafe fn deserialize_from_file(
         engine: &impl AsEngineRef,
         path: impl AsRef<Path>,
     ) -> Result<Self, DeserializeError> {
         let artifact = engine
             .as_engine_ref()
             .engine()
             .0
-            .deserialize_from_file_checked(path.as_ref())?;
+            .deserialize_from_file(path.as_ref())?;
         Ok(Self::from_artifact(artifact))
     }
 

lib/cache/src/filesystem.rs

@@ -103,7 +103,7 @@ impl Cache for FileSystemCache {
             key.to_string()
         };
         let path = self.path.join(filename);
-        let ret = Module::deserialize_from_file_checked(engine, path.clone());
+        let ret = Module::deserialize_from_file(engine, path.clone());
         if ret.is_err() {
             // If an error occurs while deserializing then we can not trust it anymore
             // so delete the cache file

lib/cli/src/commands/run.rs

@@ -472,7 +472,7 @@ impl RunWithPathBuf {
         if wasmer_compiler::Artifact::is_deserializable(&contents) {
             let engine = wasmer_compiler::EngineBuilder::headless();
             let store = Store::new(engine);
-            let module = Module::deserialize_from_file_checked(&store, &self.path)?;
+            let module = unsafe { Module::deserialize_from_file(&store, &self.path)? };
             return Ok((store, module));
         }
         let (store, compiler_type) = self.store.get_store()?;

lib/compiler/src/engine/artifact.rs

@@ -143,12 +143,15 @@ impl Artifact {
         ))
     }
 
-    /// Deserialize a ArtifactBuild
+    /// Deserialize a serialized artifact.
     ///
     /// # Safety
-    /// This function is unsafe because rkyv reads directly without validating
-    /// the data.
-    pub fn deserialize_checked(engine: &Engine, bytes: &[u8]) -> Result<Self, DeserializeError> {
+    /// This function loads executable code into memory.
+    /// You must trust the loaded bytes to be valid for the chosen engine and
+    /// for the host CPU architecture.
+    /// In contrast to [`Self::deserialize_unchecked`] the artifact layout is
+    /// validated, which increases safety.
+    pub unsafe fn deserialize(engine: &Engine, bytes: &[u8]) -> Result<Self, DeserializeError> {
         if !ArtifactBuild::is_deserializable(bytes) {
             return Err(DeserializeError::Incompatible(
                 "Magic header not found".to_string(),
@@ -161,18 +164,25 @@ impl Artifact {
         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_checked(metadata_slice)?;
+        let serializable = SerializableModule::deserialize(metadata_slice)?;
         let artifact = ArtifactBuild::from_serializable(serializable);
         let mut inner_engine = engine.inner_mut();
         Self::from_parts(&mut inner_engine, artifact, engine.target())
             .map_err(DeserializeError::Compiler)
     }
 
-    /// Deserialize a ArtifactBuild
+    /// Deserialize a serialized artifact.
+    ///
+    /// NOTE: You should prefer [`Self::deserialize`].
     ///
     /// # Safety
-    /// This function is unsafe because rkyv reads directly without validating the data.
-    pub unsafe fn deserialize(engine: &Engine, bytes: &[u8]) -> Result<Self, DeserializeError> {
+    /// See [`Self::deserialize`].
+    /// In contrast to the above, this function skips artifact layout validation,
+    /// which increases the risk of loading invalid artifacts.
+    pub unsafe fn deserialize_unchecked(
+        engine: &Engine,
+        bytes: &[u8],
+    ) -> Result<Self, DeserializeError> {
         if !ArtifactBuild::is_deserializable(bytes) {
             let static_artifact = Self::deserialize_object(engine, bytes);
             match static_artifact {