Soundness issues with Ptr

[ammaraskar]

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed that some soundness issues in the Ptr class.

---

1. Ptr implements the Send and Sync traits for all types:

cgc/src/mem.rs

Lines 829 to 830 in e8e277f

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

This allows you to send types that aren't safe to use across threads like Rc and Cell. We'd recommend only implementing this when the T: Send and T: Sync.

---

2. Ptr violates aliasing rules by having .get() take a reference and return a mutable reference.

cgc/src/mem.rs

Lines 760 to 764 in e8e277f

	impl<T: ?Sized> Ptr<T> {
	pub fn get(&self) -> &mut T {
	unsafe { &mut *self.0 }
	}
	}

This allows you to create multiple mutable references to the same object which is undefined behavior in Rust.

---

3. Ptr::write writes to the raw pointer underneath:

cgc/src/mem.rs

Lines 775 to 777 in e8e277f

	pub fn set(&self, val: T) {
	unsafe { self.0.write(val) };
	}

This can lead to data-races, using an atomic pointer would be better if you want to support multiple threads.

---

Here's a proof-of-concept for the two of the issues above:

#![forbid(unsafe_code)]

use cgc::mem::Ptr;
use std::rc::Rc;

fn wild_sync() {
    // 1. Wild Send and Sync
    let rc = Rc::new(42);
    let ptr = Ptr::new(rc.clone());

    std::thread::spawn(move || {
        let smuggled_rc = ptr.take();

        println!("Thread: {:p}", smuggled_rc);
        for _ in 0..100_000_000 {
            smuggled_rc.clone();
        }
    });

    println!("Main:   {:p}", rc);
    for _ in 0..100_000_000 {
        rc.clone();
    }
}

// A simple tagged union used to demonstrate problems with aliasing.
#[derive(Debug, Clone, Copy)]
enum RefOrInt { Ref(&'static u64), Int(u64) }

fn aliasing() {
    // 2. Aliasing violation
    let ptr = Ptr::new(RefOrInt::Ref(&42));

    let mutable_ref_one = ptr.get();
    let mutable_ref_two = ptr.get();

    println!("Pointer points to: {:?}", mutable_ref_one);
    if let RefOrInt::Ref(ref addr) = mutable_ref_one {
        *mutable_ref_two = RefOrInt::Int(0xdeadbeef);

        println!("Pointer now points to: {:p}", *addr);
        println!("Dereferencing addr will now segfault: {}", **addr);
    }
}

fn main() {
    wild_sync();
    aliasing();
}

This outputs:

Pointer points to: Ref(42)
Pointer now points to: 0xdeadbeef

Terminated with signal 11 (SIGSEGV)

[Shnatsel]

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.

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.