Skip to content

Commit

Permalink
Rework Module deserialize methods
Browse files Browse the repository at this point in the history
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
  • Loading branch information
theduke committed May 23, 2023
1 parent 06f7308 commit 402e115
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 6 deletions.
22 changes: 16 additions & 6 deletions lib/api/src/jsc/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,18 +184,28 @@ impl Module {
}

pub unsafe fn deserialize_unchecked(
_engine: &impl AsEngineRef,
_bytes: impl IntoBytes,
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
Self::deserialize(engine, bytes)
}

pub unsafe fn deserialize(
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
return Self::from_binary(_engine, &_bytes.into_bytes())
.map_err(|e| DeserializeError::Compiler(e));
}

pub unsafe fn deserialize(
_engine: &impl AsEngineRef,
_bytes: impl IntoBytes,
pub unsafe fn deserialize_from_file_unchecked(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
unimplemented!();
let bytes = std::fs::read(path.as_ref())?;
Self::deserialize_unchecked(engine, bytes)
}

pub unsafe fn deserialize_from_file_unchecked(
Expand Down
31 changes: 31 additions & 0 deletions lib/compiler/src/engine/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,37 @@ impl Artifact {
/// 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(),
));
}

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(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 serialized artifact.
///
/// NOTE: You should prefer [`Self::deserialize`].
///
/// # Safety
/// 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 {
Expand Down

0 comments on commit 402e115

Please sign in to comment.