Passing slices to JS allows mutating arbitrary WASM memory

[Liamolucko]

Describe the Bug

&[u8] can be passed to JS, in the form of a Uint8Array slice into WASM memory. This can then be mutated from JS, kept after the lifetime of the slice, or passed back to Rust - effectively creating a safe version of Uint8Array::view.

Steps to Reproduce

use js_sys::Uint8Array;
use wasm_bindgen::prelude::*;
use wasm_bindgen_test::*;

// wasm-bindgen-test uses node, so this had to be commonjs
#[wasm_bindgen(inline_js = "exports.identity = x => x")]
extern "C" {
    // This is effectively a safe version of `Uint8Array::view`, which shouldn't be able to exist.
    #[wasm_bindgen(js_name = identity)]
    fn safe_view(slice: &[u8]) -> Uint8Array;
}

fn mutate_immutable_slice(slice: &[u8]) {
    let view = safe_view(slice);
    view.set_index(1, 5);
}

#[wasm_bindgen_test]
fn oh_no() {
    let array = [1, 2, 3, 4];
    mutate_immutable_slice(&array);
    // Mutation through an immutable reference - UB!
    assert_eq!(array, [1, 5, 3, 4]);
}

Expected Behavior

It shouldn't be possible to mutate through an immutable reference, particularly not after the lifetime of the reference ends. Either the ability to pass slices to JS should be removed, or the contents of the slice should be copied.

To be fair, JS can already modify arbitrary WASM memory, but you'd have to be explicitly accessing the raw memory and working with raw pointers. With this, seemingly innocuous code can cause undefined behaviour.

Actual Behavior

Passing an immutable slice to JS gives JS a permanent, mutable view of that region of WASM memory.

Additional Context

I was looking at #1187, and thinking it would actually be sound to return a value of any lifetime to WASM, since to be able to return it at all that type would have to explicitly implement IntoWasmAbi for any lifetime. But then I saw that the implementation for slices would create a view into WASM memory, and that they could be safely passed to JS by using an elided lifetime on the imported function.

[alexcrichton]

Thanks for the report! This is sort of both a feature and a bug of wasm-bindgen. Wasm-bindgen itself doesn't know whether passing a slice to a JS function is supposed to be fast or not, so we pessimistically assume they all need to be fast and share direct views of wasm memory as a result. This, however, leads to the consequences you're seeing where there's no protection if wasm memory grows or against mutation from JS (since we can't otherwise prevent mutation).

Overall this decision was made to be pragmatic at the time (speed is good! sometimes...) but is not a great long-term solution. In the future this is what interface-types for WebAssembly is for where that has crisp definitions of all the types and semantic meanings about them, so it's clear that what wasm-bindgen does today would not suffice for those APIs and behavior would need to change.

In the interim though this is a "feature" of wasm-bindgen that I don't think can change. It's an unfortunate feature and one that should be fixed, but I think it's best fixed with the next evolutionary stage of wasm-bindgen (something interface-types based)