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

Workaround for a rustc bug that can currently lead to UB #63

Merged
merged 1 commit into from
May 17, 2023

Conversation

kyren
Copy link
Owner

@kyren kyren commented May 16, 2023

Credit goes to me for stumbling across it and 100% to @moulins for actually finding the fix.

I do not have a link to an open rust issue for the specific bug, I can try and find a link later if @moulins doesn't already know which one it is.

@moulins
Copy link
Collaborator

moulins commented May 16, 2023

As extra evidence that this is an actual rustc bug and not an issue with gc-arena's abstractions, here's a piece of code using a similar trait setup and Any to demonstrate UB with no unsafe code (playground link):

use std::any::Any;
use std::sync::OnceLock;

trait Visit {
    fn visit(&self);
}

trait Visitable<'a> {
    type Visit: Visit + 'a;
}

static HOLDER: OnceLock<&'static String> = OnceLock::new();

impl<T> Visit for &'static T {
    fn visit(&self) {
        let any = (*self) as &dyn Any;
        if let Some(i) = any.downcast_ref::<&'static String>() {
            HOLDER.get_or_init(|| i);
        }
    }
}

fn call_and_visit<V, F>(value: String, f: F)
where
    V: for<'a> Visitable<'a> + ?Sized,
    F: for<'a> Fn(&'a String) -> <V as Visitable<'a>>::Visit,
{
    f(&value).visit();
}

type MyVisitable = dyn for<'a> Visitable<'a, Visit = &'static &'a String>;

fn main() {
    call_and_visit::<MyVisitable, _>("hello".into(), |i| {
        Box::leak(Box::new(i))
    });

    // oops, use-after-free
    dbg!(HOLDER.get());
}

I believe this is an instance of rust-lang/rust#84533, as it also involves "broken" dyn Trait types allowing the bypass of well-formed checks.

@kyren kyren merged commit 5c5e925 into master May 17, 2023
@kyren kyren deleted the soundness-fix branch July 8, 2024 20:46
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 this pull request may close these issues.

2 participants