wasm3 / wasm3-rs

Rust wrapper for Wasm3, the fastest WebAssembly interpreter
MIT License
155 stars 43 forks source link

`ParsedModule` API is unsound #25

Closed jonas-schievink closed 3 years ago

jonas-schievink commented 3 years ago

According to https://github.com/wasm3/wasm3/blob/8850f91c46510ca44097e8b1d55441bc4de544e5/source/wasm3.h#L218-L222, the WASM bytecode passed to m3_ParseModule needs to stay alive during the module's lifetime, but the wasm3-rs API does not enforce this with a lifetime parameter. The following code causes a segmentation fault:

use std::fs;

use wasm3::{Environment, Function, ParsedModule, Runtime};

fn open() -> Result<Runtime, Box<dyn std::error::Error>> {
    let wasm = fs::read("empty.wasm")?;

    let env = Environment::new()?;
    let parsed_module = ParsedModule::parse(&env, &wasm)?;

    let rt = Runtime::new(&env, 1024 * 16)?;
    rt.load_module(parsed_module)?;

    Ok(rt)
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let rt = open()?;

    let call_func: Function<(i32, i32, i32), i32> = rt.find_function("_call")?;
    let ret = call_func.call(0, 0, 0)?;
    assert_eq!(ret, 0);
    drop(call_func);

    Ok(())
}

where empty.wasm is empty.wasm.gz (but most WASM files should reproduce the problem).

Veykril commented 3 years ago

👋😄 I can fix that up, any preferences in whether the module should always borrow the data or would be nicer usage wise to have the module take ownership/clone the bytes?

jonas-schievink commented 3 years ago

:wave: In hotg-ai/rune#255 it would be a lot easier to have it take ownership instead of borrowing

Veykril commented 3 years ago

Just released a 0.2.0 version that should fix this by taking Into<Box<[u8]>> and keeping the data alive until the runtime drops(since module unloading isnt implemented yet thats the only way to free it for now).