Skip to content

`cell-project` used incorrect variance when projecting through `&Cell<T>`

Moderate severity GitHub Reviewed Published Sep 16, 2022 to the GitHub Advisory Database • Updated Jan 11, 2023

Package

cargo cell-project (Rust)

Affected versions

< 0.1.4

Patched versions

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<T> is invariant in T,
so casting to *const T relaxed the variance, and lead to unsoundness, as shown in the example below.

use std::cell::Cell;
use cell_project::cell_project as cp;

struct Foo<'a> {
    x: Option<&'a Cell<Foo<'a>>>,
}

impl<'a> Drop for Foo<'a> {
    fn drop(&mut self) {
        // `ourselves` is an &Cell<Self>.
        // 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

$ cargo +nightly miri run
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `<snip>`
error: Undefined Behavior: not granting access to tag <untagged> because incompatible item is protected: [Unique for <2472> (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 <untagged> because incompatible item is protected: [Unique for <2472> (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::<Foo>::replace` at $RUST_STD_PATH/src/rust/library/core/src/cell.rs:404:31
note: inside `<Foo as std::ops::Drop>::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::<Foo> - shim(Some(Foo))` at $RUST_STD_PATH/src/rust/library/core/src/ptr/mod.rs:486:1
    = note: inside `std::ptr::drop_in_place::<std::cell::UnsafeCell<Foo>> - shim(Some(std::cell::UnsafeCell<Foo>))` at $RUST_STD_PATH/src/rust/library/core/src/ptr/mod.rs:486:1
    = note: inside `std::ptr::drop_in_place::<std::cell::Cell<Foo>> - shim(Some(std::cell::Cell<Foo>))` 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

References

Published to the GitHub Advisory Database Sep 16, 2022
Reviewed Sep 16, 2022
Last updated Jan 11, 2023

Severity

Moderate

Weaknesses

No CWEs

CVE ID

No known CVE

GHSA ID

GHSA-p75v-367r-2v23
Loading Checking history
See something to contribute? Suggest improvements for this vulnerability.