Skip to content

Commit

Permalink
feat: Implement validation for serialized modules
Browse files Browse the repository at this point in the history
Implement validation of rykv serialized modules with the "validation"
feature which uses bytecheck.

Bumps the MAGIC VERSION of the serialization header , because some
format changes were required.
(mostly setting an explicit repr() for enums)

Also updates various APIs to add unsafe methods as
"dangerous_deserialize_unchecked", deprecate the old unsafe "deserialize" methods
and to add new "deserialize_checked" methods
  • Loading branch information
theduke committed Mar 30, 2023
1 parent 4ccbc4b commit 6c7d172
Show file tree
Hide file tree
Showing 30 changed files with 531 additions and 57 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions lib/api/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ impl Engine {
///
/// The serialized content must represent a serialized WebAssembly module.
pub unsafe fn deserialize(&self, bytes: &[u8]) -> Result<Arc<Artifact>, DeserializeError> {
Ok(Arc::new(Artifact::deserialize(&self.0, bytes)?))
Ok(Arc::new(Artifact::dangerous_deserialize_unchecked(
&self.0, bytes,
)?))
}

#[deprecated(since = "3.2.0")]
Expand All @@ -157,7 +159,10 @@ impl Engine {
let mut buffer = Vec::new();
// read the whole file
file.read_to_end(&mut buffer)?;
Ok(Arc::new(Artifact::deserialize(&self.0, buffer.as_slice())?))
Ok(Arc::new(Artifact::dangerous_deserialize_unchecked(
&self.0,
buffer.as_slice(),
)?))
}

#[deprecated(since = "3.2.0", note = "Use Engine::deterministic_id()")]
Expand Down
20 changes: 20 additions & 0 deletions lib/api/src/js/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,18 @@ 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(
_engine: &impl AsEngineRef,
_bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
#[cfg(feature = "js-serializable-module")]
return Self::from_binary(_engine, &_bytes.into_bytes())
.map_err(|e| DeserializeError::Compiler(e));

#[cfg(not(feature = "js-serializable-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(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
Expand All @@ -246,6 +258,14 @@ impl Module {
Self::deserialize(engine, bytes)
}

pub fn deserialize_from_file_checked(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
let bytes = std::fs::read(path.as_ref())?;
Self::deserialize_checked(engine, bytes)
}

pub fn set_name(&mut self, name: &str) -> bool {
self.name = Some(name.to_string());
true
Expand Down
94 changes: 92 additions & 2 deletions lib/api/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,21 @@ impl Module {
Ok(())
}

/// See [`Self::dangerous_deserialize_unchecked`].
///
/// # Safety
/// See [`Self::dangerous_deserialize_unchecked`].
#[deprecated(
since = "3.2.0",
note = "Use `deserialize_checked` or `dangerous_deserialize_unchecked` instead"
)]
pub unsafe fn deserialize(
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
Ok(Self(module_imp::Module::deserialize(engine, bytes)?))
}

/// Deserializes a serialized Module binary into a `Module`.
///
/// # Important
Expand Down Expand Up @@ -246,13 +261,88 @@ impl Module {
/// # Ok(())
/// # }
/// ```
pub unsafe fn deserialize(
pub unsafe fn dangerous_deserialize_unchecked(
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
Ok(Self(module_imp::Module::deserialize(engine, bytes)?))
}

/// Deserializes a serialized Module binary into a `Module`.
///
/// # Important
///
/// This function only accepts a custom binary format, which will be different
/// than the `wasm` binary format and may change among Wasmer versions.
/// (it should be the result of the serialization of a Module via the
/// `Module::serialize` method.).
///
/// # Safety
///
/// This function is inherently **unsafe** as the provided bytes:
/// 1. Are going to be deserialized directly into Rust objects.
/// 2. Contains the function assembly bodies and, if intercepted,
/// a malicious actor could inject code into executable
/// memory.
///
/// And as such, the `deserialize` method is unsafe.
///
/// # Usage
///
/// ```ignore
/// # use wasmer::*;
/// # fn main() -> anyhow::Result<()> {
/// # let mut store = Store::default();
/// let module = Module::deserialize(&store, serialized_data)?;
/// # Ok(())
/// # }
/// ```
pub fn deserialize_checked(
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
Ok(Self(module_imp::Module::deserialize_checked(
engine, bytes,
)?))
}

/// See [`Self::dangerous_deserialize_from_file_unchecked`].
///
/// # Safety
/// See [`Self::dangerous_deserialize_from_file_unchecked`].
#[deprecated(
since = "3.2.0",
note = "Use `deserialize_from_file_checked` or `dangerous_deserialize_from_file_unchecked` instead"
)]
pub unsafe fn deserialize_from_file(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
Self::dangerous_deserialize_from_file_unchecked(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.
///
/// # Usage
///
/// ```ignore
/// # use wasmer::*;
/// # let mut store = Store::default();
/// # fn main() -> anyhow::Result<()> {
/// let module = Module::deserialize_from_file(&store, path)?;
/// # Ok(())
/// # }
/// ```
pub fn deserialize_from_file_checked(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
Ok(Self(module_imp::Module::deserialize_from_file_checked(
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.
///
Expand All @@ -270,7 +360,7 @@ impl Module {
/// # Ok(())
/// # }
/// ```
pub unsafe fn deserialize_from_file(
pub unsafe fn dangerous_deserialize_from_file_unchecked(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
Expand Down
33 changes: 31 additions & 2 deletions lib/api/src/sys/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,24 @@ impl Module {
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
let bytes = bytes.into_bytes();
let artifact = engine.as_engine_ref().engine().0.deserialize(&bytes)?;
let artifact = engine
.as_engine_ref()
.engine()
.0
.dangerous_deserialize_unchecked(&bytes)?;
Ok(Self::from_artifact(artifact))
}

pub fn deserialize_checked(
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)?;
Ok(Self::from_artifact(artifact))
}

Expand All @@ -86,7 +103,19 @@ impl Module {
.as_engine_ref()
.engine()
.0
.deserialize_from_file(path.as_ref())?;
.dangerous_deserialize_from_file_unchecked(path.as_ref())?;
Ok(Self::from_artifact(artifact))
}

pub fn deserialize_from_file_checked(
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())?;
Ok(Self::from_artifact(artifact))
}

Expand Down
91 changes: 74 additions & 17 deletions lib/compiler/src/engine/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,51 @@ impl 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> {
if !ArtifactBuild::is_deserializable(bytes) {
return Err(DeserializeError::Incompatible(
"Magic header not found".to_string(),
));
}

let bytes = Self::get_byte_slice(bytes, ArtifactBuild::MAGIC_HEADER.len(), bytes.len())?;

let metadata_len = MetadataHeader::parse(bytes)?;
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 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)
}

/// See [`Self::dangerous_deserialize_unchecked`].
///
/// # Safety
/// See [`Self::dangerous_deserialize_unchecked`].
// TODO: remove in 4.0
#[deprecated(
since = "3.2.0",
note = "Use `deserialize_checked` or `dangerous_deserialize_unchecked` instead"
)]
pub unsafe fn deserialize(engine: &Engine, bytes: &[u8]) -> Result<Self, DeserializeError> {
Self::dangerous_deserialize_unchecked(engine, bytes)
}

/// Deserialize a ArtifactBuild
///
/// NOTE: This function can introduce undefined behaviour!
/// You should usually use [`Self::deserialize_checked`] instead.
///
/// # Safety
/// This function is unsafe because rkyv reads directly without validating
/// the data.
pub unsafe fn dangerous_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 {
Expand All @@ -170,7 +214,7 @@ 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(metadata_slice)?;
let serializable = SerializableModule::dangerous_deserialize_unchecked(metadata_slice)?;
let artifact = ArtifactBuild::from_serializable(serializable);
let mut inner_engine = engine.inner_mut();
Self::from_parts(&mut inner_engine, artifact, engine.target())
Expand Down Expand Up @@ -714,20 +758,6 @@ impl Artifact {
))
}

/// Deserialize a ArtifactBuild from an object file
///
/// # Safety
/// The object must be a valid static object generated by wasmer.
#[cfg(not(feature = "static-artifact-load"))]
pub unsafe fn deserialize_object(
_engine: &Engine,
_bytes: &[u8],
) -> Result<Self, DeserializeError> {
Err(DeserializeError::Compiler(
CompileError::UnsupportedFeature("static load is not compiled in".to_string()),
))
}

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)
Expand All @@ -741,19 +771,46 @@ impl Artifact {
}
}

/// See [`Self::dangerous_deserialize_unchecked`].
///
/// # Safety
/// See [`Self::dangerous_deserialize_unchecked`].
pub unsafe fn deserialize_object(
engine: &Engine,
bytes: &[u8],
) -> Result<Self, DeserializeError> {
Self::dangerous_deserialize_object_unchecked(engine, bytes)
}

/// Deserialize a ArtifactBuild from an object file
///
/// # Safety
/// The object must be a valid static object generated by wasmer.
#[cfg(not(feature = "static-artifact-load"))]
#[allow(unused_variables)]
pub unsafe fn dangerous_deserialize_object_unchecked(
engine: &Engine,
bytes: &[u8],
) -> Result<Self, DeserializeError> {
Err(DeserializeError::Compiler(
CompileError::UnsupportedFeature("static load is not compiled in".to_string()),
))
}

/// Deserialize a ArtifactBuild from an object file
///
/// # Safety
/// The object must be a valid static object generated by wasmer.
#[cfg(feature = "static-artifact-load")]
pub unsafe fn deserialize_object(
pub unsafe fn dangerous_deserialize_object_unchecked(
engine: &Engine,
bytes: &[u8],
) -> Result<Self, DeserializeError> {
let metadata_len = MetadataHeader::parse(bytes)?;
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)?;
let metadata: ModuleMetadata =
ModuleMetadata::dangerous_deserialize_unchecked(metadata_slice)?;

const WORD_SIZE: usize = mem::size_of::<usize>();
let mut byte_buffer = [0u8; WORD_SIZE];
Expand Down
Loading

0 comments on commit 6c7d172

Please sign in to comment.