GHSA-P75V-367R-2V23

Vulnerability from github – Published: 2022-09-16 21:08 – Updated: 2022-09-16 21:08
VLAI?
Summary
`cell-project` used incorrect variance when projecting through `&Cell<T>`
Details

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

Show details on source website
To clipboard 

{
  "affected": [
    {
      "package": {
        "ecosystem": "crates.io",
        "name": "cell-project"
      },
      "ranges": [
        {
          "events": [
            {
              "introduced": "0"
            },
            {
              "fixed": "0.1.4"
            }
          ],
          "type": "ECOSYSTEM"
        }
      ]
    }
  ],
  "aliases": [],
  "database_specific": {
    "cwe_ids": [],
    "github_reviewed": true,
    "github_reviewed_at": "2022-09-16T21:08:19Z",
    "nvd_published_at": null,
    "severity": "MODERATE"
  },
  "details": "## Overview\n\nThe issue lies in the implementation of the `cell_project` macro which used `field as *const _` instead of `field as *mut _`.\nThe problem being that `*const T` is covariant in `T` while `*mut T` is invariant in `T`. Keep in mind that `\u0026Cell\u003cT\u003e` is invariant in `T`,\nso casting to `*const T` relaxed the variance, and lead to unsoundness, as shown in the example below.\n\n```rs\nuse std::cell::Cell;\nuse cell_project::cell_project as cp;\n\nstruct Foo\u003c\u0027a\u003e {\n    x: Option\u003c\u0026\u0027a Cell\u003cFoo\u003c\u0027a\u003e\u003e\u003e,\n}\n\nimpl\u003c\u0027a\u003e Drop for Foo\u003c\u0027a\u003e {\n    fn drop(\u0026mut self) {\n        // `ourselves` is an \u0026Cell\u003cSelf\u003e.\n        // NB: `Drop` is unsound.\n        if let Some(ourselves) = self.x.as_ref() {\n            // replace `self` (but this doesn\u0027t actually replace `self`)\n            let is_x_none = ourselves.replace(Foo {\n                x: None,\n            }).x.as_ref().is_none();\n            // if we just moved out of `self`, and we had a `Some` originally,\n            // how come this is a `None`?\n            if is_x_none {\n                println!(\"how did we get a None?\");\n            }\n        }\n    }\n}\n\nfn main() {\n    let foo = Cell::new(Foo {\n        x: None,\n    });\n    let x = cp!(Foo\u003c\u0027_\u003e, foo.x);\n    x.set(Some(\u0026foo));\n}\n```\n\n### MIRI error\n\n```rs\n$ cargo +nightly miri run\n    Finished dev [unoptimized + debuginfo] target(s) in 0.01s\n     Running `\u003csnip\u003e`\nerror: Undefined Behavior: not granting access to tag \u003cuntagged\u003e because incompatible item is protected: [Unique for \u003c2472\u003e (call 796)]\n   --\u003e $RUST_STD_PATH/src/rust/library/core/src/cell.rs:404:31\n    |\n404 |         mem::replace(unsafe { \u0026mut *self.value.get() }, val)\n    |                               ^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag \u003cuntagged\u003e because incompatible item is protected: [Unique for \u003c2472\u003e (call 796)]\n    |\n    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental\n    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information\n\n    = note: inside `std::cell::Cell::\u003cFoo\u003e::replace` at $RUST_STD_PATH/src/rust/library/core/src/cell.rs:404:31\nnote: inside `\u003cFoo as std::ops::Drop\u003e::drop` at src/main.rs:14:29\n   --\u003e src/main.rs:14:29\n    |\n14  |               let is_x_none = ourselves.replace(Foo {\n    |  _____________________________^\n15  | |                 x: None,\n16  | |             }).x.as_ref().is_none();\n    | |______________^\n    = note: inside `std::ptr::drop_in_place::\u003cFoo\u003e - shim(Some(Foo))` at $RUST_STD_PATH/src/rust/library/core/src/ptr/mod.rs:486:1\n    = note: inside `std::ptr::drop_in_place::\u003cstd::cell::UnsafeCell\u003cFoo\u003e\u003e - shim(Some(std::cell::UnsafeCell\u003cFoo\u003e))` at $RUST_STD_PATH/src/rust/library/core/src/ptr/mod.rs:486:1\n    = note: inside `std::ptr::drop_in_place::\u003cstd::cell::Cell\u003cFoo\u003e\u003e - shim(Some(std::cell::Cell\u003cFoo\u003e))` at $RUST_STD_PATH/src/rust/library/core/src/ptr/mod.rs:486:1\nnote: inside `main` at src/main.rs:32:1\n   --\u003e src/main.rs:32:1\n    |\n32  | }\n    | ^\n\nnote: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace\n\nerror: aborting due to previous error\n```\n\n## Affected Versions\n\nAll versions of the cell-project crate before `0.1.4` are affected.\n\n## Mitigation\n\nThis was fixed in [Issues/4], and released as version `0.1.4`.\nSo just updating to the latest version will include the fix (which may result in a compile error on unsound usage).\n\n## Acknowledgements\n\nThis was discovered and fixed by @SoniEx2 in cell-project: [Issues/3] and [Issues/4]\n\n[issues/3]: https://github.com/RustyYato/cell-project/issues/3\n[issues/4]: https://github.com/RustyYato/cell-project/issues/4\n",
  "id": "GHSA-p75v-367r-2v23",
  "modified": "2022-09-16T21:08:19Z",
  "published": "2022-09-16T21:08:19Z",
  "references": [
    {
      "type": "WEB",
      "url": "https://github.com/RustyYato/cell-project/issues/3"
    },
    {
      "type": "WEB",
      "url": "https://github.com/RustyYato/cell-project/pull/4"
    },
    {
      "type": "PACKAGE",
      "url": "https://github.com/RustyYato/cell-project"
    },
    {
      "type": "WEB",
      "url": "https://rustsec.org/advisories/RUSTSEC-2020-0164.html"
    }
  ],
  "schema_version": "1.4.0",
  "severity": [],
  "summary": "`cell-project` used incorrect variance when projecting through `\u0026Cell\u003cT\u003e`"
}


Log in or create an account to share your comment.




Tags
Taxonomy of the tags.




Sightings

Author Source Type Date

Nomenclature

  • Seen: The vulnerability was mentioned, discussed, or observed by the user.
  • Confirmed: The vulnerability has been validated from an analyst's perspective.
  • Published Proof of Concept: A public proof of concept is available for this vulnerability.
  • Exploited: The vulnerability was observed as exploited by the user who reported the sighting.
  • Patched: The vulnerability was observed as successfully patched by the user who reported the sighting.
  • Not exploited: The vulnerability was not observed as exploited by the user who reported the sighting.
  • Not confirmed: The user expressed doubt about the validity of the vulnerability.
  • Not patched: The vulnerability was not observed as successfully patched by the user who reported the sighting.



Detection rules are retrieved from Rulezet.