Skip to content

Rust invalid free in MetaCallException::new and other unsoundness issues #809

@thanasistrisp

Description

@thanasistrisp

Hello! I found several memory related vulnerabilities I would like to report in https://crates.io/crates/metacall crate at latest version 0.5.10.

Memory corruption in MetaCallException::new / Drop

#[test]
fn exception_bad_free_safe_api() {
    let original = metacall::MetaCallException::new(
        "test",
        "test",
        "test",
        1,
    );

    drop(original);
}

Running this test with ASAN, I got the following report:

METACALL_INSTALL_PATH=/path/to/metacall-install \
LD_LIBRARY_PATH=/path/to/metacall-install/lib:$LD_LIBRARY_PATH \
RUSTFLAGS="-Zsanitizer=address" \
cargo +nightly test --test metacall_exception_test exception_bad_free_safe_api -- --nocapture

SUMMARY: AddressSanitizer: bad-free /core/source/reflect/source/reflect_exception.c:330:4 in exception_destroy
==3120305==ABORTING

exception_struct is a local stack variable, but the code passes its address to C: &mut exception_struct as *mut _ as *mut c_void.
Then the returned MetaCall value is stored here:

Ok(Self {
    exception_struct: Arc::new(exception_struct),
    value: exception_ptr,
    leak: false,
})

Because leak is false, the destructor will run later. But the original exception pointer points to Rust stack memory. Address sanitizer reports bad-free.

Possible solution: the struct should be heap allocated if it is intended to pass on C.

For compiling with ASAN, because the repro does not use inline macros, I locally disabled that module in src/lib.rs:116:

#[cfg(any())]
pub mod inline {
    pub use metacall_inline::*;
}

Other issues (unsoundness)

The following issues are referring to src/types/metacall_exception.rs but very similar issues should exist to src/types/metacall_class.rs, src/types/metacall_future.rs, src/types/metacall_pointer.rs, src/issues/metacall_function.rs, src/issues/metacall_object.rs, src/event/event_handler.rs and src/load.rs.

  • Clone traits seem dangerous because value is shallow copied and leak=true does not guarantee safety; Clone does not free the MetaCall value, but it still stores the same raw pointer. If the original is dropped, the clone can retain a dangling pointer (value).
  • new_raw method is a safe function that accepts arbitrary raw pointer and dereferences C memory. This function is only correct if the caller gives it a valid, owned MetaCall value that must be destroyed by this wrapper. The method should be even internal and not exposed to the public API or be declared as unsafe (and be correctly documented).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions