Skip to content

Commit

Permalink
Prevent file system access when loading known modules with the same s…
Browse files Browse the repository at this point in the history
…pecifier as before
  • Loading branch information
Arshia001 committed Sep 20, 2024
1 parent b803c1d commit 3a43123
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 19 deletions.
4 changes: 2 additions & 2 deletions ion/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ pub trait ModuleLoader {
) -> crate::Result<Module<'cx>>;

/// Registers a new module in the module registry. Useful for native modules.
fn register(&mut self, cx: &Context, module: &Object, request: &ModuleRequest) -> crate::Result<()>;
fn register(&mut self, cx: &Context, module: &Object, request: String) -> crate::Result<()>;

/// Returns metadata of a module, used to populate `import.meta`.
fn metadata(&self, cx: &Context, data: Option<&ModuleData>, meta: &mut Object) -> crate::Result<()>;
Expand All @@ -218,7 +218,7 @@ impl ModuleLoader for () {
Err(Error::new("Modules are unsupported by this loader.", None))
}

fn register(&mut self, _: &Context, _: &Object, _: &ModuleRequest) -> crate::Result<()> {
fn register(&mut self, _: &Context, _: &Object, _: String) -> crate::Result<()> {
Ok(())
}

Expand Down
48 changes: 36 additions & 12 deletions runtime/src/module/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,16 @@ impl ModuleLoader for Loader {
) -> ion::Result<Module<'cx>> {
let specifier = request.specifier(cx).to_owned(cx)?;

// Do a registry look-up before canonicalizing paths, since the
// canonicalization process is incompatible with built-in modules
// that don't have an address on disk.
if let Some(heap) = self.registry.get(&specifier) {
return Ok(Module::from_local(heap.root(cx)));
// If the request is for a built-in module (signified by having a : character
// in the specifier) look it up now and fail if we don't have it.
if specifier.contains(':') {
return match self.registry.get(&specifier) {
Some(heap) => Ok(Module::from_local(heap.root(cx))),
None => Err(ion::Error::new(
format!("Cannot find built-in module {specifier}"),
ion::ErrorKind::Type,
)),
};
}

let path = if !specifier.starts_with('/') {
Expand All @@ -46,6 +51,14 @@ impl ModuleLoader for Loader {
Path::new(&specifier).to_path_buf()
};

// Perform a look-up of the uncanonicalized path of the module. This helps
// when the module was loaded before but its file is no longer available,
// such as when resuming a pre-initialized WASM binary.
let uncanonicalized_path_string = String::from(path.to_str().unwrap());
if let Some(heap) = self.registry.get(&uncanonicalized_path_string) {
return Ok(Module::from_local(heap.root(cx)));
}

let path = canonicalize_path(&path).or_else(|e| {
if path.extension() == Some(OsStr::new("js")) {
return Err(e);
Expand All @@ -65,9 +78,17 @@ impl ModuleLoader for Loader {
canonicalize_path(&parent.join(file_name))
})?;

let str = String::from(path.to_str().unwrap());
match self.registry.get(&str) {
Some(heap) => Ok(Module::from_local(heap.root(cx))),
let path_string = String::from(path.to_str().unwrap());
match self.registry.get(&path_string) {
Some(heap) => {
let module_obj = Object::from(heap.root(cx));
if path_string != uncanonicalized_path_string {
// Register the module under the new specifier as well
// to keep future lookups happy
self.register(cx, &module_obj, uncanonicalized_path_string)?;
}
Ok(Module::from_local(module_obj.into_local()))
}
None => {
let script = read_to_string(&path).map_err(|e| {
Error::new(
Expand All @@ -93,8 +114,12 @@ impl ModuleLoader for Loader {
let module = Module::compile(cx, &specifier, Some(path.as_path()), &script);

if let Ok(module) = module {
let request = ModuleRequest::new(cx, path.to_str().unwrap());
self.register(cx, module.module_object(), &request)?;
// Register the module under both the canonical path and the specifier,
// so that we find in both of these situations:
// * when a different import call refers to the same module file with a different specifier
// * when re-importing with the same specifier, without needing to touch the file system
self.register(cx, module.module_object(), uncanonicalized_path_string)?;
self.register(cx, module.module_object(), path_string)?;
Ok(module)
} else {
Err(Error::new(format!("Unable to compile module: {}\0", specifier), None))
Expand All @@ -103,8 +128,7 @@ impl ModuleLoader for Loader {
}
}

fn register(&mut self, cx: &Context, module: &Object, request: &ModuleRequest) -> ion::Result<()> {
let specifier = request.specifier(cx).to_owned(cx)?;
fn register(&mut self, _cx: &Context, module: &Object, specifier: String) -> ion::Result<()> {
match self.registry.entry(specifier) {
Entry::Vacant(v) => {
v.insert(TracedHeap::from_local(module));
Expand Down
9 changes: 4 additions & 5 deletions runtime/src/module/standard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use ion::{Context, Object};
use ion::flags::PropertyFlags;
use ion::module::{Module, ModuleRequest};
use ion::module::Module;

pub trait StandardModules {
fn init(self, cx: &Context, global: &Object) -> bool;
Expand Down Expand Up @@ -51,10 +51,9 @@ pub fn init_module<M: NativeModule>(cx: &Context, global: &Object) -> bool {
if global.define_as(cx, internal, &module, PropertyFlags::CONSTANT) {
let module = Module::compile(cx, M::NAME, None, M::SOURCE).unwrap();
let loader = unsafe { &mut (*cx.get_inner_data().as_ptr()).module_loader };
return loader.as_mut().is_some_and(|loader| {
let request = ModuleRequest::new(cx, M::NAME);
loader.register(cx, module.module_object(), &request).is_ok()
});
return loader
.as_mut()
.is_some_and(|loader| loader.register(cx, module.module_object(), M::NAME.to_owned()).is_ok());
}
}
false
Expand Down

0 comments on commit 3a43123

Please sign in to comment.