[Bug]: Unsoundness in u8_to_u32

[lwz23]

What happened?

A bug happened! I notice the following code:

pub fn u8_to_u32(asset_data: Arc<Vec<u8>>) -> Vec<u32> {
    let mut words = vec![0u32; asset_data.len() / std::mem::size_of::<u32>()];
    unsafe {
        std::ptr::copy_nonoverlapping(
            asset_data.as_ptr(),
            words.as_mut_ptr() as *mut u8,
            asset_data.len(),
        );
    }

    words
}

I think it will result a unaligned problem. Poc

use std::sync::Arc;

pub fn u8_to_u32(asset_data: Arc<Vec<u8>>) -> Vec<u32> {
    let mut words = vec![0u32; asset_data.len() / std::mem::size_of::<u32>()];
    unsafe {
        std::ptr::copy_nonoverlapping(
            asset_data.as_ptr(),
            words.as_mut_ptr() as *mut u8,
            asset_data.len(),
        );
    }

    words
}

fn main() {
    let mut data: Vec<u8> = vec![0; 17]; 

    for i in 0..17 {
        data[i] = i as u8;
    }

    let asset_data = Arc::new(data);

    let result = u8_to_u32(asset_data);

    println!("{:?}", result);
}

Output

PS E:\Github\lwz> cargo +nightly miri run
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s
     Running `C:\Users\ROG\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\bin\cargo-miri.exe runner target\miri\x86_64-pc-windows-msvc\debug\lwz.exe`
error: Undefined Behavior: memory access failed: expected a pointer to 17 bytes of memory, but got alloc1520 which is only 16 bytes from the end of the allocation
  --> src\main.rs:6:9
   |
6  | /         std::ptr::copy_nonoverlapping(
7  | |             asset_data.as_ptr(),
8  | |             words.as_mut_ptr() as *mut u8,
9  | |             asset_data.len(),
10 | |         );
   | |_________^ memory access failed: expected a pointer to 17 bytes of memory, but got alloc1520 which is only 16 bytes from the end of the allocation
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
help: alloc1520 was allocated here:
  --> src\main.rs:4:21
   |
4  | ... = vec![0u32; asset_data.len() / std::mem::size_of::<u32>(...
   |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: BACKTRACE (of the first span):
   = note: inside `u8_to_u32` at src\main.rs:6:9: 10:10
note: inside `main`
  --> src\main.rs:29:18
   |
29 |     let result = u8_to_u32(asset_data);
   |                  ^^^^^^^^^^^^^^^^^^^^^
   = note: this error originates in the macro `vec` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error


note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace


note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace


note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace


note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace


note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

error: process didn't exit successfully: `C:\Users\ROG\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\bin\cargo-miri.exe runner target\miri\x86_64-pc-windows-msvc\debug\lwz.exe` (exit code: 1)
PS E:\Github\lwz>

Version

0.1.1 (Default)

In which Hotham component are you seeing the problem on?

Other

What VR System are you seeing the problem on?

None

What OS are you seeing the problem on?

No response

Relevant log output

No response

[kanerogers]

Hi there! I'm so sorry you're running into this issue!

You're absolutely right that the implementation of u8_to_u32 is unsound; I really didn't do any validation there at all. But, this never turned out to be a big problem since if we're crashing, it's because you gave us something that wasn't actually a SPIR-V file.

I'd be more than happy to accept a PR that uses read_spv instead.

[lwz23]

Thanks for your quick response, you are right that this is not a serious problem. I also noticed this code because I was testing my static scanning tool to look for unsound problems in Rust :) According to rust's security requirements, it is the developer's responsibility to ensure that public functions declared safe should not generate UB under any circumstances. So I think there are two possible ways to fix it:

1. Mark the function as unsafe
2. Add checks inside the function to ensure that std::ptr::copy_nonoverlapping security preconditions are not violated

It is the programmer’s responsibility when writing unsafe code to ensure that any safe code interacting with the unsafe code cannot trigger these behaviors. unsafe code that satisfies this property for any safe client is called sound; if unsafe code can be misused by safe code to exhibit undefined behavior, it is unsound.

[kanerogers]

Absolutely! If you look at my previous comment, you'll see that I suggested that the correct solution here is to use the function provided by the ash crate, rather than trying to roll our own (poorly implemented) version.