dereference-after-free in bitbox! macro, as per Miri

[YoshikiTakashima]

Hello.

Looks like the macro bitbox! has a dereference after free bug.

As of 293e670, if you run

fn main() {
        let _ : bitvec::boxed::BitBox = bitvec::bitbox![0];
}

with cargo miri run -- -Zmiri-disable-stacked-borrows
``

error: Undefined Behavior: pointer to alloc1014 was dereferenced after this allocation got freed
   --> /home/ytakashima/.cargo/git/checkouts/bitvec-de08e8e455d8f2a6/293e670/src/boxed.rs:420:23
    |
420 |             bitptr.set_pointer(boxed.as_ptr() as *mut T);
    |                                ^^^^^ pointer to alloc1014 was dereferenced after this allocation got freed
    |
    = 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 `bitvec::boxed::BitBox::with_box::<[closure@bitvec::boxed::ops::<impl std::ops::Drop for bitvec::boxed::BitBox<O, T>>::drop::{{closure}}#0], ()>` at /home/ytakashima/.cargo/git/checkouts/bitvec-de08e8e455d8f2a6/293e670/src/boxed.rs:420:23
    = note: inside `bitvec::boxed::ops::<impl std::ops::Drop for bitvec::boxed::BitBox>::drop` at /home/ytakashima/.cargo/git/checkouts/bitvec-de08e8e455d8f2a6/293e670/src/boxed/ops.rs:149:3
    = note: inside `std::intrinsics::drop_in_place::<bitvec::boxed::BitBox> - shim(Some(bitvec::boxed::BitBox))` at /home/ytakashima/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/ptr/mod.rs:184:1
note: inside `main` at src/main.rs:2:52
   --> src/main.rs:2:52
    |
2   |     let _ : bitvec::boxed::BitBox = bitvec::bitbox![0];
    |                                                       ^
    = note: inside closure at /home/ytakashima/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:67:34
    = note: inside closure at /home/ytakashima/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:52:73
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<[closure@std::rt::lang_start_internal::{{closure}}#0::{{closure}}#0 0:&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /home/ytakashima/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/sys_common/backtrace.rs:130:5
    = note: inside closure at /home/ytakashima/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:52:13
    = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{{closure}}#0 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /home/ytakashima/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:348:40
    = note: inside `std::panicking::r#try::<i32, [closure@std::rt::lang_start_internal::{{closure}}#0 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe]>` at /home/ytakashima/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:325:15
    = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{{closure}}#0 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /home/ytakashima/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panic.rs:394:14
    = note: inside `std::rt::lang_start_internal` at /home/ytakashima/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:51:25
    = note: inside `std::rt::lang_start::<()>` at /home/ytakashima/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:67:5

error: aborting due to previous error

Looks like it's not related to any of the active issues. I haven't gotten to thinking the security implications, but there might be some.

Thanks
~Yoshi

[myrrlyn]

The stack trace shows that this is occurring during the BitBox destructor; the pointer to the allocation is being used immediately before the BitBox stack handle is completely destroyed. This does not access the referent memory, nor does it cause any observable safety issues in user code (as by now, the handle is out of scope, and inaccessible).

That said, undefined behavior under Miri is a future avenue for miscompilation. Since the function in question is an internal implementation detail used only in one other site, I can freely remove the offending behavior.

[myrrlyn]

Thank you for the report! I will release 0.18.1 with this patch included after I go through the rest of my issue backlog.

[YoshikiTakashima]

Hi @myrrlyn. I agree with you that it is the destructor. Looks like I missed that somehow.

I'm glad it is fixed easily. Thank you.