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

Soundness issues in StaticMut #5

Open
ammaraskar opened this issue Dec 22, 2020 · 3 comments
Open

Soundness issues in StaticMut #5

ammaraskar opened this issue Dec 22, 2020 · 3 comments

Comments

@ammaraskar
Copy link

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed a few soundness issues with the StaticMut type.

  1. There is an aliasing violation with as_mut

    stderr/src/static_mut.rs

    Lines 78 to 80 in 2fd8eb8

    pub fn as_mut(&self) -> &mut T {
    unsafe { self.0.get().as_mut().unwrap() }
    }

  2. It implements Sync for all types T, this should be restricted to T: Sync

    unsafe impl<T> Sync for StaticMut<T> {}

We noticed you pointed out some of these issues in the documentation, maybe it would be better to mark these methods as unsafe or restrict the visibility of StaticMut, otherwise this allows for safety violations from Rust without using unsafe. For example:

#![forbid(unsafe_code)]

use stderr::StaticMut;

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

fn main() {
    let ptr = StaticMut::new(RefOrInt::Ref(&42));

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

    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);
    }
}

causes:

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

Terminated with signal 11 (SIGSEGV)
@biluohc
Copy link
Owner

biluohc commented Dec 22, 2020

Thanks for your work, but this library should not be used anymore, because eprintln of the standard library has been stable for too long

@ammaraskar
Copy link
Author

ammaraskar commented Dec 22, 2020

Thank you for the quick response, in that case I would recommend yanking all the crate's versions so no one uses it by accident :) https://doc.rust-lang.org/cargo/commands/cargo-yank.html

@biluohc
Copy link
Owner

biluohc commented Dec 22, 2020

Not for the time being, I still have some projects that rely on it. And StaticMut itself is not designed as RwLock, Mutex or Atomic

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