ArcGuard's Send and Sync should have bounds on RC

[ammaraskar]

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed that async_coap::arc_guard::ArcGuard implements Send and Sync so long as T: Send and T: Sync.

rust-async-coap/async-coap/src/arc_guard.rs

Lines 163 to 164 in 6a7b592

	unsafe impl<RC, T: Send> Send for ArcGuard<RC, T> {}
	unsafe impl<RC, T: Sync> Sync for ArcGuard<RC, T> {}

However, this should also probably be bounded by RC: Send and RC: Sync, otherwise it's possible to smuggle across non-Send types across thread boundaries. Here's a proof-of-concept that segfaults safe rust code:

#![forbid(unsafe_code)]

use async_coap::arc_guard::ArcGuard;

use std::{cell::Cell, sync::Arc};
use crossbeam_utils::thread;

// A simple tagged union used to demonstrate problems with data races in Cell.
#[derive(Debug, Clone, Copy)]
enum RefOrInt {
    Ref(&'static u64),
    Int(u64),
}
static SOME_INT: u64 = 123;

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

    let arc_guard = ArcGuard::new(arc, |_| ());
    thread::scope(|s| {
        s.spawn(|_| {
            let smuggled_arc = (&arc_guard).head();

            loop {
                // Repeatedly write Ref(&addr) and Int(0xdeadbeef) into the cell.
                smuggled_arc.set(RefOrInt::Ref(&SOME_INT));
                smuggled_arc.set(RefOrInt::Int(0xdeadbeef));
            }
        });

        loop {
            if let RefOrInt::Ref(addr) = (**arc_guard.head()).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; }

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

Output:

Pointer is now: 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.