search

rustwasm / wasm-bindgen

Facilitating high-level interactions between Wasm modules and JavaScript
https://rustwasm.github.io/docs/wasm-bindgen/
Apache License 2.0
7.85k stars 1.08k forks source link

aborted modules must be poisoned #3687

Open workingjubilee opened 1 year ago

workingjubilee commented 1 year ago

Using wasm-bindgen, a wasm module can be resumed after abort by calling into it again from JavaScript. This violates Rust's soundness preconditions: abort must terminate forward progress.

The wasm component model prevents wasm modules from being reused in this way by locking out calls into them after a module has trapped. However, wasm bindgen runs on wasm itself, which lacks this safeguard.

It is likely that wasm-bindgen should be implementing the check that poisons calls into it, or shuts off access in any other way. Even if this were implemented upstream for all users of wasm32-unknown-unknown, it would be best for wasm-bindgen to deprecate any interfaces that serve primarily to expose this route to unsound interactions to userspace. The ability to throw JavaScript exceptions has been called out as a likely example.

Also see:

daxpedda commented 1 year ago

I left some additional questions/comments in the Zulip discussions, but to summarize: I'm not entirely convinced that this is a problem in wasm-bindgen because throwing a JS exception doesn't call abort().

RalfJung commented 1 year ago

What is the type of throwing a JS exception in Rust? AFAIK wasm is a no-unwind target. So every function that returns ! is like abort -- no code must ever be executed in this thread any more.

daxpedda commented 1 year ago

So every function that returns ! is like abort -- no code must ever be executed in this thread any more.

It is indeed !. Is this really required by Rust? It is hard to find any documentation around this. As far as I can tell this isn't practically unsafe: even though no-unwind Wasm throws, it doesn't clear the stack where it throws. Obviously this can and will cause issues, but it shouldn't be unsafe or cause UB.

If this is just a "semantic" issue, I would like to propose to change that in Rust: not require ! to be an abort(), or at least on some platforms, e.g. Wasm.

RalfJung commented 1 year ago

The concern is around optimizations Rust performs that need this rule, so it's not something we can fix by just changing some wording somewhere.

I'll need to better understand what this function does and doesn't do... once I have time I'll catch up on Zulip and have some questions there.

daxpedda commented 1 year ago

So conclusion: This is definitely unsafe and Rust should implement some sort of module poisoning when we abort.

From wasm-bindgens side, we could definitely just deprecate any throwing function, though I'm not sure if this does make sense because it's definitely a widely used and probably necessary future, so alternatively we might want to add poisoning to wasm-bindgen as well or when we add poisoning to Rust we should add it in a way that can cover issues like that.

I would prefer to wait for now, I will definitely find time to make some experiments in Rust and see how we can implement poisoning there.