Unsound impl of From<&'a [u8]> for Uint8Array

[Michael-F-Bryan]

Describe the Bug

The From<&'a [u8]> for Uint8Array impl is unsound, allowing the user to create dangling pointers.

Steps to Reproduce

Doing anything with the Uint8Array returned from this foo() function will let you access memory from foo()'s stack frame after it's been popped.

fn foo() -> Uint8Array {
   // create a buffer of bytes on the stack
  let mut buffer = [0; 256];
  // return a view into the array
  Uint8Array::from(&buffer[..])
  // the buffer is popped from the stack
}

Expected Behaviour

It should be impossible to use a dangling pointer without using unsafe in foo.

This should be prevented at compile time, or a defensive copy should be made at runtime.

Actual Behaviour

The Uint8Array lets you access bytes from a stack frame after it's been popped.

Additional Context

• This bug is most probably present for all typed arrays declared using the arrays!() macro
• This is a security vulnerability, seeing as it gives you direct access to whatever is on the stack (or WASM linear memory, I'm not 100% sure how the stack and heap are implemented in WASM)

[Michael-F-Bryan]

See also @Pauan's comment at #1619 (comment):

Be very careful when using view, it is an unsafe function for a reason: you have to immediately make a copy (without doing any Rust allocation) in order to avoid undefined behavior.

The docs for Uint8Array::view() (the From<&[u8]> impl is just a call to Uint8Array::view()):

/// Creates a JS typed array which is a view into wasm's linear
/// memory at the slice specified.
///
/// This function returns a new typed array which is a view into
/// wasm's memory. This view does not copy the underlying data.
///
/// # Unsafety
///
/// Views into WebAssembly memory are only valid so long as the
/// backing buffer isn't resized in JS. Once this function is called
/// any future calls to `Box::new` (or malloc of any form) may cause
/// the returned value here to be invalidated. Use with caution!
///
/// Additionally the returned object can be safely mutated but the
/// input slice isn't guaranteed to be mutable.
///
/// Finally, the returned object is disconnected from the input
/// slice's lifetime, so there's no guarantee that the data is read
/// at the right time.

[Michael-F-Bryan]

Sorry, Ignore me. I think this was actually a bug in my code.

Originally I had the below code and without making a second copy of the Uint8Array I was getting garbled data (i.e. it looked like a dangling pointer issue). I can't reproduce it any more so I'm guessing the problem was elsewhere.

#[wasm_bindgen]
pub fn encode_gcode_program(
    chunk_number: u16,
    first_line: u32,
    text: &str,
) -> Uint8Array {
    let mut buffer = [0; anpp::Packet::MAX_PACKET_SIZE];
    let msg = GcodeProgram::new(chunk_number, first_line, text);
    let bytes_written = buffer
        .pwrite_with(msg, 0, Endian::network())
        .expect("Will always succeed");

    Uint8Array::from(&buffer[..bytes_written])
}

[Pauan]

To be clear, this is not a security vulnerability, because JavaScript always has full 100% access to all of Rust's memory (stack and heap). So it's always possible for JavaScript to access dangling pointers and things like that.

Secondly, Uint8Array::from does make a copy, therefore it is completely safe. That's why it is not an unsafe function. This is explained in a comment in the source code.