GitHub Advisoryadvisory · crates.io

GHSA-p75v-367r-2v23 `cell-project` used incorrect variance when projecting through `&Cell<T>`

MEDIUM Published 2022-09-16 Modified 2022-09-16 Official page ↗
Ecosystem
crates.io
Packages
cell-project
Severity
Moderate
Reviewed
2022-09-16

Affected packages

• crates.io cell-project: < 0.1.4 → fixed in 0.1.4

Description

Overview The issue lies in the implementation of the `cell_project` macro which used `field as *const _` instead of `field as *mut _`. The problem being that `*const T` is covariant in `T` while `*mut T` is invariant in `T`. Keep in mind that `&Cell` is invariant in `T`, so casting to `*const T` relaxed the variance, and lead to unsoundness, as shown in the example below. ```rs use std::cell::Cell; use cell_project::cell_project as cp; struct Foo { x: Option>>, } impl Drop for Foo { fn drop(&mut self) { // `ourselves` is an &Cell. // NB: `Drop` is unsound. if let Some(ourselves) = self.x.as_ref() { // replace `self` (but this doesn't actually replace `self`) let is_x_none = ourselves.replace(Foo { x: None, }).x.as_ref().is_none(); // if we just moved out of `self`, and we had a `Some` originally, // how come this is a `None`? if is_x_none { println!("how did we get a None?"); } } } } fn main() { let foo = Cell::new(Foo { x: None, }); let x = cp!(Foo, foo.x); x.set(Some(&foo)); } ``` MIRI error ```rs $ cargo +nightly miri run Finished dev [unoptimized + debuginfo] target(s) in 0.01s Running `` error: Undefined Behavior: not granting access to tag because incompatible item is protected: [Unique for (call 796)] --> $RUST_STD_PATH/src/rust/library/core/src/cell.rs:404:31 | 404 | mem::replace(unsafe { &mut *self.value.get() }, val) | ^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because incompatible item is protected: [Unique for (call 796)] | = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information = note: inside `std::cell::Cell::::replace` at $RUST_STD_PATH/src/rust/library/core/src/cell.rs:404:31 note: inside `::drop` at src/main.rs:14:29 --> src/main.rs:14:29 | 14 | let is_x_none = ourselves.replace(Foo { | _____________________________^ 15 | | x: None, 16 | | }).x.as_ref().is_none(); | |______________^ = note: inside `std::ptr::drop_in_place:: - shim(Some(Foo))` at $RUST_STD_PATH/src/rust/library/core/src/ptr/mod.rs:486:1 = note: inside `std::ptr::drop_in_place::> - shim(Some(std::cell::UnsafeCell))` at $RUST_STD_PATH/src/rust/library/core/src/ptr/mod.rs:486:1 = note: inside `std::ptr::drop_in_place::> - shim(Some(std::cell::Cell))` at $RUST_STD_PATH/src/rust/library/core/src/ptr/mod.rs:486:1 note: inside `main` at src/main.rs:32:1 --> src/main.rs:32:1 | 32 | } | ^ note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace error: aborting due to previous error ``` Affected Versions All versions of the cell-project crate before `0.1.4` are affected. Mitigation This was fixed in [Issues/4], and released as version `0.1.4`. So just updating to the latest version will include the fix (which may result in a compile error on unsound usage). Acknowledgements This was discovered and fixed by @SoniEx2 in cell-project: [Issues/3] and [Issues/4] [issues/3]: https://github.com/RustyYato/cell-project/issues/3 [issues/4]: https://github.com/RustyYato/cell-project/issues/4

References