VecCopy allows for misaligned access

[ammaraskar]

Hey there, I noticed that in VecCopy the backing storage for the Vec is a u8.

dync/src/vec_copy.rs

Lines 64 to 65 in c133056

	/// Raw data stored as bytes.
	pub(crate) data: Vec<MaybeUninit<u8>>,

I believe this let's you trigger undefined behavior in the form of misaligned memory access through safe rust code by instantiating a VecCopy with a type that has different alignment requirements from u8. Running the following program under miri:

#![forbid(unsafe_code)]

use dync::{VecCopy, VTable};

#[repr(align(256))]
#[derive(Copy, Clone)]
struct LargeAlign(u8);

impl VTable<LargeAlign> for LargeAlign {
    fn build_vtable() -> Self {
        LargeAlign(0)
    }
}

fn main() {
    // The backing storage for a VecCopy is a u8, meaning that casting to a type
    // with different alginment requires triggers undefined behavior.
    // https://github.com/elrnv/dync/blob/c133056676582dd0e28c14526175d0c9ae01a905/src/vec_copy.rs#L64-L65
    let mut x = VecCopy::<LargeAlign>::with_type();
    x.push_as::<LargeAlign>(LargeAlign(0));

    let _ref_to_element = x.get_ref_as::<LargeAlign>(0).unwrap();
}

results in:

❯ cargo miri run
error: Undefined Behavior: accessing memory with alignment 8, but alignment 256 is required
   --> /home/ammar/.cargo/registry/src/github.com-1ecc6299db9ec823/dync-0.4.0/src/vec_copy.rs:523:23
    |
523 |         Some(unsafe { &*ptr.add(i) })
    |                       ^^^^^^^^^^^^ accessing memory with alignment 8, but alignment 256 is required
    |
    = 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
            
    = note: inside `dync::VecCopy::<LargeAlign>::get_ref_as::<LargeAlign>` at /home/ammar/.cargo/registry/src/github.com-1ecc6299db9ec823/dync-0.4.0/src/vec_copy.rs:523:23
note: inside `main` at src/main.rs:41:27
   --> src/main.rs:41:27
    |
41  |     let _ref_to_element = x.get_ref_as::<LargeAlign>(0).unwrap();
    |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:137:18
    = note: inside closure at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:66:18
    = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
    = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:381:40
    = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:345:19
    = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:382:14
    = note: inside `std::rt::lang_start_internal` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:51:25
    = note: inside `std::rt::lang_start::<()>` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:65:5

error: aborting due to previous error

[elrnv]

Thank you! I've been meaning to get rid of the unchecked Vec casts. This is a great motivator :)

[elrnv]

Fixed in 6972f94 and published in version 0.5.
The presented example is included as is in a dedicated integration test in tests/soundness.rs.
Additionally CI is updated to include a miri run on all the tests to prevent further regressions.
However, for the record, implementing the VTable trait for the contained type is unnecessary --- the following example would have sufficed:

#![forbid(unsafe_code)]

use dync::{VecCopy};

#[repr(align(256))]
#[derive(Copy, Clone)]
struct LargeAlign(u8);

fn main() {
    let mut x: VecCopy = VecCopy::with_type::<LargeAlign>();
    x.push_as::<LargeAlign>(LargeAlign(0));

    let _ref_to_element = x.get_ref_as::<LargeAlign>(0).unwrap();
}