allocator-api2 icon indicating copy to clipboard operation
allocator-api2 copied to clipboard

Box is unsound

Open glandium opened this issue 2 years ago • 4 comments

allocator-api2's Box is backed by a NonNull. std's Box is backed by a Unique. Unique is a wrapper around NonNull with an important addition: a marker to give a hint to dropck

The Box type in this crate doesn't have this marker.

Relatedly, I'm working on refreshing https://crates.io/crates/allocator-api with code automatically generated from the code in the rust repo (and comparing what I have vs this crate is how I found this discrepancy).

glandium avatar Jan 12 '24 00:01 glandium

Thank you for reporting this.

Can you provide an example that will cause UB with allocator_api2::Box without unsafe?

I guess the fix is adding PhantomData<T> here

zakarumych avatar Jan 13 '24 12:01 zakarumych

See https://stackoverflow.com/questions/42708462/why-is-it-useful-to-use-phantomdata-to-inform-the-compiler-that-a-struct-owns-a/42721125#42721125

glandium avatar Jan 14 '24 00:01 glandium

The current Drop impl on Box does not indicate a #[may_dangle] on T, so I do not quite see that the linked question and answer applies currently - although a proper fix and the insertion of such a #[may_dangle] would most be nice to have.

WorldSEnder avatar Jan 29 '24 17:01 WorldSEnder

My memory is hazy wrt details (it was 6 years ago), but I'm rather sure that I've ended up with cases where the borrow checker would allow things leading to UAFs when I wrote the initial Box in the allocator-api crate without a PhantomData, and that was without may_dangle because it's never been stable.

OTOH, I can't reproduce with trivial test cases with the rustc version of the time (1.25.0, which is the first version with NonNull in libstd). That said, I'd rather be overly cautious than not enough.

glandium avatar Sep 10 '24 08:09 glandium