mirror of
https://github.com/fluencelabs/wasm-bindgen
synced 2025-03-16 02:00:51 +00:00
Fix use-after-free with closures in JS bindings
This commit fixes an erroneous use-after-free which can happen in erroneous situations in JS. It's intended that if you invoke a closure after its environment has been destroyed that you'll immediately get an error from Rust saying so. The JS binding generation for mutable closures, however, accidentally did not protect against this. Each closure has an internal reference count which is incremented while being invoked and decremented when the invocation finishes and also when the `Closure` in Rust is dropped. That means there's two branches where the reference count reaches zero and the internal pointer stored in JS needs to be set to zero. Only one, however, actually set the pointer to zero! This means that if a closure was destroyed while it was being invoked it would not correctly set its internal pointer to zero. A further invocation of the closure would then pass as seemingly valid pointer into Rust, causing a use-after-free. A test isn't included here specifically for this because our CI has started failing left-and-right over this test, so this commit will hopefully just make our CI green!
This commit is contained in:
parent
b8e9a20d8f
commit
2ff2e2505a
@ -181,18 +181,36 @@ impl ClosureDescriptors {
|
||||
let mut shim = closure.shim_idx;
|
||||
let (js, _ts, _js_doc) = {
|
||||
let mut builder = Js2Rust::new("", input);
|
||||
|
||||
// First up with a closure we increment the internal reference
|
||||
// count. This ensures that the Rust closure environment won't
|
||||
// be deallocated while we're invoking it.
|
||||
builder.prelude("this.cnt++;");
|
||||
|
||||
if closure.mutable {
|
||||
// For mutable closures they can't be invoked recursively.
|
||||
// To handle that we swap out the `this.a` pointer with zero
|
||||
// while we invoke it. If we finish and the closure wasn't
|
||||
// destroyed, then we put back the pointer so a future
|
||||
// invocation can succeed.
|
||||
builder
|
||||
.prelude("let a = this.a;\n")
|
||||
.prelude("this.a = 0;\n")
|
||||
.prelude("let a = this.a;")
|
||||
.prelude("this.a = 0;")
|
||||
.rust_argument("a")
|
||||
.rust_argument("b")
|
||||
.finally("this.a = a;\n");
|
||||
.finally("if (this.cnt-- == 1) d(a, b);")
|
||||
.finally("else this.a = a;");
|
||||
} else {
|
||||
builder.rust_argument("this.a").rust_argument("b");
|
||||
// For shared closures they can be invoked recursively so we
|
||||
// just immediately pass through `this.a`. If we end up
|
||||
// executing the destructor, however, we clear out the
|
||||
// `this.a` pointer to prevent it being used again the
|
||||
// future.
|
||||
builder
|
||||
.rust_argument("this.a")
|
||||
.rust_argument("b")
|
||||
.finally("if (this.cnt-- == 1) { d(this.a, b); this.a = 0; }");
|
||||
}
|
||||
builder.finally("if (this.cnt-- == 1) d(this.a, b);");
|
||||
builder.process(&closure.function, None)?.finish(
|
||||
"function",
|
||||
"f",
|
||||
|
Loading…
x
Reference in New Issue
Block a user