Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible soundness issue in Shared? #3

Open
ammaraskar opened this issue Nov 10, 2020 · 1 comment
Open

Possible soundness issue in Shared? #3

ammaraskar opened this issue Nov 10, 2020 · 1 comment

Comments

@ammaraskar
Copy link

Hi there, we (the Rust group at @sslab-gatech) are auditing crates on crates.io for safety issues and noticed that the Shared object seems to not have bounds on its Send/Sync traits:

model/src/lib.rs

Lines 110 to 112 in b50d9ee

unsafe impl<T> Sync for Shared<T> {}
unsafe impl<T> Send for Shared<T> {}

We weren't sure if this was intentional since this is a testing library but this can be used to create data races from safe Rust by sharing types like Cell:

#![forbid(unsafe_code)]

use model::Shared;

use std::cell::Cell;
use crossbeam_utils::thread;

#[derive(Debug, Clone, Copy)]
enum RefOrInt<'a> { Ref(&'a u64), Int(u64) }

static SOME_INT: u64 = 123;

fn main() {
    let cell = Cell::new(RefOrInt::Ref(&SOME_INT));
    let shared = Shared::new(&cell);

    thread::scope(|s| {
        s.spawn(move |_| {
            loop {
                // Repeatedly write Ref(&addr) and Int(0xdeadbeef) into the cell.
                shared.set(RefOrInt::Ref(&SOME_INT));
                shared.set(RefOrInt::Int(0xdeadbeef));
            }
        });

        loop {
            if let RefOrInt::Ref(addr) = cell.get() {
                // Hope that between the time we pattern match the object as a
                // `Ref`, it gets written to by the other thread.
                if addr as *const u64 == &SOME_INT as *const u64 {
                    continue;
                }

                // Due to the data race, obtaining Ref(0xdeadbeef) is possible
                println!("Pointer is now: {:p}", addr);
                println!("Dereferencing addr will now segfault: {}", *addr);
            }
        }
    });
}

which outputs:

Pointer is now: 0xdeadbeef

Return Code: -11 (SIGSEGV)
@Shnatsel
Copy link

Heads up: this issue has been included in the RustSec advisory database. It will be surfaced by tools such as cargo-audit or cargo-deny from now on.

Since the project is intended for testing rather than production environments, it is surfaced as a warning rather than an error.

Once a fix is released to crates.io, please open a pull request to update the advisory with the patched version, or file an issue on the advisory database repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants