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

Rhai is unsound (use after free) #894

Open
devzf opened this issue Jul 3, 2024 · 5 comments
Open

Rhai is unsound (use after free) #894

devzf opened this issue Jul 3, 2024 · 5 comments
Labels

Comments

@devzf
Copy link

devzf commented Jul 3, 2024

Consider the following example:

fn main() {
    let global_name: std::rc::Rc<std::cell::RefCell<&'static str>> = Default::default();
    let global_name2 = global_name.clone();
    {
        let mut engine = rhai::Engine::new();

        engine.register_fn("hello", move |name: &'static str| {
            *global_name.borrow_mut() = name;
        });

        engine.eval::<()>(r#"hello("some name")"#).unwrap();
    }

    println!("{:?}", &*global_name2.borrow());
}

rhai v1.19.0

@schungx
Copy link
Collaborator

schungx commented Jul 4, 2024

I'm outside right now but on the surface it looks legit... Why do you say therr is OB?

@schungx
Copy link
Collaborator

schungx commented Jul 4, 2024

Ah I see it now. The 'static is throwing it off...

Sneaky...

I must have a way to distinguish between &str and &'static str...

@schungx schungx added the bug label Jul 4, 2024
@schungx
Copy link
Collaborator

schungx commented Jul 5, 2024

So far I have not been able to find a way to avoid the user passing in &'static str... short of disallowing &str altogether which will seriously break code.

That's because, to the Rust compiler, lifetimes do not form part of a type's ID. Therefore, it merrily thinks that &'static str and &str are the same type.

@schungx schungx pinned this issue Jul 6, 2024
@Cerber-Ursi
Copy link

Do you have any reason to allow registered functions take any non-'static value at all? It seems that this use-after-free is possible with any borrowed value, not just &str.

@schungx
Copy link
Collaborator

schungx commented Jul 10, 2024

Well, non-'static lifetimes are OK because Rhai doesn't return any lifetime. That's why passing in a normal &str is absolutely OK because within the Rust function there is nothing you can do to abuse that reference with a lifetime -- you cannot store it in anything provided by Rhai because all the functions are + 'static.

Therefore, I'm free to assume that, as long as the reference passed into the function outlives the function call, everything is OK.

Except that if that reference is 'static, then suddenly it satisfies the Rhai function requirements and you can now store those references inside Rhai variables. That's what caused the error.

It is only with &str because that's specially treated by Rhai for special handling... it gets converted to a reference inside an ImmutableString for convenience. Other references I suppose are simply never passed anything because there is no data in Rhai that is stored with references (except for the special treatment for ImmutableString). For any other type the function is simply never called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants