wasmtime / issue #10031 Possible Memory Corruption From L... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #10031 Possible Memory Corruption From L...

Wasmtime GitHub notifications bot (Jan 16 2025 at 11:06):

Ridwanul-OpenRefactory opened issue #10031:

In file: options.rs, function new, a parameter is borrowed in a return value. But the lifetime of the parameter doesn't match the lifetime of the return value. It may cause memory bugs like Use After Free or Non Exclusive Mutability in several places in the code.
    pub unsafe fn new(
        store: StoreContextMut<'a, T>,
        options: &'a Options,
        types: &'a ComponentTypes,
        instance: *mut ComponentInstance,
    ) -> LowerContext<'a, T> {
        LowerContext {
            store,
            options,
            types,
            instance,
        }
    }
The parameter instance is a raw pointer inside the return value, but its lifetime is not compatible with the lifetime ('a) of the return value. Since instance is a raw pointer, it is not subject to Rust's borrow checker. The compiler does not enforce borrowing rules for raw pointers, such as ensuring no data races or aliasing for mutable pointers, or checking the validity of the memory they point to. As a result, instance may outlive the lifetime of the return value, leading to potential memory bugs like use-after-free or non-exclusive mutability

How to access the raw pointer to trigger use-after-free:
    pub unsafe fn bar(
        store: StoreContextMut<'a, T>,
        options: &'a Options,
        types: &'a ComponentTypes,
        instance: *mut ComponentInstance,
    ) -> LowerContext<'a, T> {
        // Convert the raw pointer into a Box to take ownership
        let boxed_instance = Box::from_raw(instance);

        // Drop the boxed instance to deallocate its memory
        drop(boxed_instance);

        // Calling the `new` function may cause memory bug
    }
Here, we add a function bar where the Box::from_raw(instance) call takes ownership of the raw pointer. Calling drop(boxed_instance) explicitly deallocates the memory that instance points to. If we create a new LowerContext object, the instance will have dangling pointer.

An example to demonstrate the impact:

Let us consider this example code that is identical to the mentioned code snippet-
struct Data<'a> {
    x: &'a str,
    y: *mut String,
}

fn bar<'a>(arg1: &'a String, arg2: *mut String) -> Data<'a> {
    // Data has max life-time 'a
    Data {
        x: arg1,
        y: arg2,
    }
}

fn foo() {
    let v1 = "Hello".to_string();
    let mut v2 = "World".to_string();
    let bar_obj = bar(&v1, &mut v2);
    // We dropped v2 but it is assigned inside bar_obj because of lifetime issues
    drop(v2);
    unsafe{
        // This is where the use after free happens
        println!("Value of v2: {}", *bar_obj.y)
    }
}
Output:
Value of v2: +�r{
As we can see in the output, v2 has garbage value. It is because we have dropped v2 but it is assigned inside the bar_obj. The drop function explicitly deallocates v2, meaning the memory that v2 pointed to is freed. However, the raw pointer still points to this freed memory which causes use-after-free bug when we try to access the y field of bar_obj.

Similarly, the instance parameter of the new function can be manifested for memory bugs.

Possible Solution:

A possible solution is to ensure that the instance parameter has a safe lifetime tied to the lifetime 'a of the return object LowerContext<'a, T> :
pub unsafe fn new(
        store: StoreContextMut<'a, T>,
        options: &'a Options,
        types: &'a ComponentTypes,
-       instance: *mut ComponentInstance,
+       instance: &'a ComponentInstance,
    ) -> LowerContext<'a, T> {
The function signature now enforces that instance must have the same lifetime as 'a and will be valid for as long as LowerContext<'a, T> exists.

Sponsorship and Support:

This work is done by the security researchers from OpenRefactory under the Project Clean Beach initiative. The OpenRefactory researchers are proactively scanning open source critical open source projects and finding previously undetected bugs in them. The goal of Project Clean Beach is to improve global software supply chain security.

The bug is found by running the iCR tool by OpenRefactory, Inc. and then manually triaging the results.

Wasmtime GitHub notifications bot (Jan 16 2025 at 11:17):

bjorn3 commented on issue #10031:

and then manually triaging the results.

Clearly not enough given https://github.com/rust-lang/cargo/issues/15039.

The compiler does not enforce borrowing rules for raw pointers, such as ensuring no data races or aliasing for mutable pointers, or checking the validity of the memory they point to. As a result, instance may outlive the lifetime of the return value, leading to potential memory bugs like use-after-free or non-exclusive mutability

The safety invariant of unsafe fn new says that this must not happen. All callers of new are required to manually prove that the safety invariant is upheld. Unless you can show a case where a caller of new doesn't uphold the safety invariant of new there is nothing to be learnt from this bullshit LLM generated report.

Wasmtime GitHub notifications bot (Jan 16 2025 at 15:25):

alexcrichton closed issue #10031:

In file: options.rs, function new, a parameter is borrowed in a return value. But the lifetime of the parameter doesn't match the lifetime of the return value. It may cause memory bugs like Use After Free or Non Exclusive Mutability in several places in the code.
    pub unsafe fn new(
        store: StoreContextMut<'a, T>,
        options: &'a Options,
        types: &'a ComponentTypes,
        instance: *mut ComponentInstance,
    ) -> LowerContext<'a, T> {
        LowerContext {
            store,
            options,
            types,
            instance,
        }
    }
The parameter instance is a raw pointer inside the return value, but its lifetime is not compatible with the lifetime ('a) of the return value. Since instance is a raw pointer, it is not subject to Rust's borrow checker. The compiler does not enforce borrowing rules for raw pointers, such as ensuring no data races or aliasing for mutable pointers, or checking the validity of the memory they point to. As a result, instance may outlive the lifetime of the return value, leading to potential memory bugs like use-after-free or non-exclusive mutability

How to access the raw pointer to trigger use-after-free:
    pub unsafe fn bar(
        store: StoreContextMut<'a, T>,
        options: &'a Options,
        types: &'a ComponentTypes,
        instance: *mut ComponentInstance,
    ) -> LowerContext<'a, T> {
        // Convert the raw pointer into a Box to take ownership
        let boxed_instance = Box::from_raw(instance);

        // Drop the boxed instance to deallocate its memory
        drop(boxed_instance);

        // Calling the `new` function may cause memory bug
    }
Here, we add a function bar where the Box::from_raw(instance) call takes ownership of the raw pointer. Calling drop(boxed_instance) explicitly deallocates the memory that instance points to. If we create a new LowerContext object, the instance will have dangling pointer.

An example to demonstrate the impact:

Let us consider this example code that is identical to the mentioned code snippet-
struct Data<'a> {
    x: &'a str,
    y: *mut String,
}

fn bar<'a>(arg1: &'a String, arg2: *mut String) -> Data<'a> {
    // Data has max life-time 'a
    Data {
        x: arg1,
        y: arg2,
    }
}

fn foo() {
    let v1 = "Hello".to_string();
    let mut v2 = "World".to_string();
    let bar_obj = bar(&v1, &mut v2);
    // We dropped v2 but it is assigned inside bar_obj because of lifetime issues
    drop(v2);
    unsafe{
        // This is where the use after free happens
        println!("Value of v2: {}", *bar_obj.y)
    }
}
Output:
Value of v2: +�r{
As we can see in the output, v2 has garbage value. It is because we have dropped v2 but it is assigned inside the bar_obj. The drop function explicitly deallocates v2, meaning the memory that v2 pointed to is freed. However, the raw pointer still points to this freed memory which causes use-after-free bug when we try to access the y field of bar_obj.

Similarly, the instance parameter of the new function can be manifested for memory bugs.

Possible Solution:

A possible solution is to ensure that the instance parameter has a safe lifetime tied to the lifetime 'a of the return object LowerContext<'a, T> :
pub unsafe fn new(
        store: StoreContextMut<'a, T>,
        options: &'a Options,
        types: &'a ComponentTypes,
-       instance: *mut ComponentInstance,
+       instance: &'a ComponentInstance,
    ) -> LowerContext<'a, T> {
The function signature now enforces that instance must have the same lifetime as 'a and will be valid for as long as LowerContext<'a, T> exists.

Sponsorship and Support:

This work is done by the security researchers from OpenRefactory under the Project Clean Beach initiative. The OpenRefactory researchers are proactively scanning open source critical open source projects and finding previously undetected bugs in them. The goal of Project Clean Beach is to improve global software supply chain security.

The bug is found by running the iCR tool by OpenRefactory, Inc. and then manually triaging the results.

Wasmtime GitHub notifications bot (Jan 16 2025 at 15:25):

alexcrichton commented on issue #10031:

Please remove Wasmtime (and other Bytecode Alliance) projects from your automatic scans. Not only is this an inaccurate report which looks like it's generated by an LLM but it doesn't follow our security policy.

Wasmtime GitHub notifications bot (Jan 16 2025 at 15:25):

alexcrichton edited a comment on issue #10031:

Please remove Wasmtime (and other Bytecode Alliance projects) from your automatic scans. Not only is this an inaccurate report which looks like it's generated by an LLM but it doesn't follow our security policy.

Wasmtime GitHub notifications bot (Jan 22 2025 at 07:03):

Ridwanul-OpenRefactory commented on issue #10031:

Thanks a lot for evaluating the bug report and giving us advice on how to better triage the bug.

We did not use any LLMs to generate the finding (LLMs cannot analyze at that level) or the bug report. We have been doing this successfully for the past 18 months (300+ bugs found in critical OSS projects) for Java, Python and Go and with our tool expanding into Rust, we have just started doing this in Rust.

Clearly, from our initial feedback, our tool has to get better at detecting and we have to get better at manually vetting. In the other case referenced, our tool was not able to capture the concept of owned, which we have now fixed. In this case, we also did not pick the context correctly. With your guidance, in future we will be more careful about reporting such bugs.

Thanks again for the feedback.

Last updated: Jan 24 2025 at 00:11 UTC