Deku's array initialization is UB, causing crashes for [Vec<T>; N] (and more).

[wildbook]

Minimal example:

use deku::{DekuContainerRead, DekuRead};

#[derive(DekuRead, Debug)]
struct Foo(#[deku(count = "0")] [Vec<u8>; 1]);

fn main() {
    Foo::from_bytes((&[0], 0)).unwrap();
}

Release builds exit with STATUS_ILLEGAL_INSTRUCTION.
Debug builds exit with STATUS_ACCESS_VIOLATION or STATUS_HEAP_CORRUPTION.

Miri output:

error: Undefined Behavior: type validation failed at .value[0].buf.ptr.pointer.pointer: encountered uninitialized raw pointer
   --> \deku-0.13.0\src\impls\slice.rs:186:46
    |
186 |             let mut slice: [T; N] = unsafe { MaybeUninit::uninit().assume_init() };
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .value[0].buf.ptr.pointer.pointer: encountered uninitialized raw pointer
    |
    = 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 `deku::impls::slice::const_generics_impl::<impl deku::DekuRead<(deku::ctx::Limit<(), for<'r> fn(&'r ()) -> bool>, ())> for [std::vec::Vec<()>; 1]>::read` at \deku-0.13.0\src\impls\slice.rs:186:46

Tested (and accidentally found) on x86_64-pc-windows-msvc @ nightly-2022-06-08, deku 0.13.0.

[wildbook]

The comment next to the unsafe block (here) states that "This is safe because we initialize the array immediately after, and never return it in case of error", but the assumption made is invalid since types that implements Drop will have T::drop called at the end of the scope, with uninitialized data.

As extensively documented in the MaybeUninit docs, MaybeUninit::uninit().assume_init() is more or less always UB. One of the very few non-UB types T for read as-is is MaybeUninit<U>, and anyone currently using Deku to parse arrays containing "normal" types is triggering UB. Including [u8; N], which is probably relatively common.

---

So, as mentioned above, it's worth noting that this affects all cases and not just the failure case. It's for example entirely possible to call drop on uninitialized data without noticing:

use deku::{DekuContainerRead, DekuRead};

fn main() {
    std::mem::forget(Foo::from_bytes((&[0u8; 50], 0)).unwrap());
}

#[derive(DekuRead, Debug)]
struct Foo([Bar; 3]);

#[derive(DekuRead, Debug)]
struct Bar(u128);

impl Drop for Bar {
    fn drop(&mut self) {
        println!("uninitialized data: {:x?}", self);
    }
}

uninitialized data: Bar(c8000007ff60a962570)
uninitialized data: Bar(7ff60a96257000007ff60a9280bf)
uninitialized data: Bar(e63ffef2580000000000000c80)

[sharksforarms]

Hi @wildbook, thanks for the report. Are you able to confirm this PR https://github.com/sharksforarms/deku/pull/254 fixes the issue? I'll get a release out asap if that's the case. If not, we'll need to make some more changes.

[wildbook]

I can confirm that the changes look good to both me and Miri, and they fix all failing examples I had locally.

Thanks for the quick fix (and for Deku in general)!

[sharksforarms]

Released in 0.13.1

Backported fix to 0.12 series as 0.12.6 and yanked affected versions (0.13.0, 0.12.5, 0.12.4)

Thanks @wildbook