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

TCell is unsound due to covariant Q parameter of TCellOwner (and the same applies to TLCell) #20

Closed
steffahn opened this issue Sep 18, 2021 · 4 comments · Fixed by #21
Closed

Comments

@steffahn
Copy link
Contributor

use qcell::{TCell, TCellOwner};

type T1 = fn(&());
type T2 = fn(&'static ());

// T1 subtype of T2, both 'static

// TCellOwner covariant
fn _demo(x: TCellOwner<T1>) -> TCellOwner<T2> {
    x
}

// and that's obviously bad

fn main() {
    let first_owner = TCellOwner::<T2>::new();
    let mut second_owner = TCellOwner::<T1>::new() as TCellOwner<T2>;

    let mut x = TCell::<T2, _>::new(vec!["Hello World!".to_owned()]);
    let reference = &first_owner.ro(&x)[0];
    second_owner.rw(&x).clear();

    println!("{}", reference); // ��&d��i
                               // (or similar output)
}
@steffahn steffahn changed the title TCell unsound due to covariant Q parameter (and the same applies to TLCell) TCell is unsound due to covariant Q parameter of TCellOwner (and the same applies to TLCell) Sep 18, 2021
@steffahn
Copy link
Contributor Author

steffahn commented Sep 18, 2021

Similar problem with covariance of Q for the cell itself

use qcell::{TCell, TCellOwner};

type T1 = fn(&());
type T2 = fn(&'static ());

// T1 subtype of T2, both 'static

fn main() {
    let first_owner = TCellOwner::<T1>::new();
    let mut second_owner = TCellOwner::<T2>::new();

    let mut x = TCell::<T1, _>::new(vec!["Hello World!".to_owned()]);
    let reference = &first_owner.ro(&x)[0];
    second_owner.rw(&x as &TCell::<T2, Vec<_>>).clear();

    println!("{}", reference); // ��&d��i
                               // (or similar output)
}

@steffahn
Copy link
Contributor Author

AFAICT, all published versions of qcell are affected.

@uazu
Copy link
Owner

uazu commented Sep 18, 2021

Thanks for spotting this. So I guess this is because Rust will cast the type, but gives it a different type-ID. I will go through your PR later and make sure I fully understand the issue and the solution. Thanks again.

@uazu uazu closed this as completed in #21 Sep 20, 2021
@uazu
Copy link
Owner

uazu commented Sep 21, 2021

Fixed in version 0.4.3. All previous versions have been yanked. I think it's unlikely anyone is stuck on pre-0.4 versions. Let me know if this causes someone a problem.

Note for anyone trying to understand the issue: this bug does not affect non-malicious programmers. Existing non-malicious code remains sound. By fixing this bug and yanking all other affected versions all that we're doing is potentially stopping a malicious coder from creating undefined behaviour without using unsafe, and sneaking bad effects through a safety review.

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

Successfully merging a pull request may close this issue.

2 participants