Skip to content

fix(rs_port): correct MetaCallException ownership and cloning - #948

Open
AshutoshSharma-pixel wants to merge 1 commit into
metacall:developfrom
AshutoshSharma-pixel:fix/rs-port-metacall-exception-safety
Open

AshutoshSharma-pixel wants to merge 1 commit into
metacall:developfrom
AshutoshSharma-pixel:fix/rs-port-metacall-exception-safety

Conversation

@AshutoshSharma-pixel

Copy link
Copy Markdown
Contributor

Summary

Fixes #809 (partially, focusing specifically on MetaCallException memory safety and RUSTSEC-2026-0156).

This PR fixes severe memory-safety vulnerabilities in the Rust port's MetaCallException wrapper:

  1. Bad-Free & Stack-Address Escape in MetaCallException::new():
    • Previously, MetaCallException::new() created a stack-allocated metacall_exception_type struct and passed its pointer to metacall_value_create_exception(&mut exception_struct as *mut _ as *mut c_void). When the stack frame exited, the C exception retained a dangling pointer to escaped stack memory. When dropped, C called free() on stack addresses, causing undefined behavior, bad-free aborts, and memory corruption.
    • Replaced stack construction with the verified heap-backed C error API metacall_error_throw(), formatting the error message safely via "%s".
  2. Independent Ownership & Extraction:
    • metacall_error_throw() returns an owned temporary throwable value wrapping the exception.
    • We extract the borrowed inner exception value via metacall_throwable_value(), deep-copy it using metacall_value_copy() (which allocates a new value header and increments the C exception reference count), and immediately destroy the temporary throwable with metacall_value_destroy().
    • Every failure path is guarded: if throwable conversion or extraction returns null, the temporary throwable is freed before panicking, eliminating any potential leaks.
  3. Correct Struct Handling in new_raw():
    • Replaced incorrect direct casting with metacall_error_from_value().
    • MetaCallException now stores owned String fields (message, label, stacktrace) rather than raw pointers into C memory, preventing use-after-free and dangling pointer access.
  4. Independent Ref-Counted Cloning & Drop:
    • Previously, Clone performed a shallow copy (self.value was duplicated while leak was set to true), causing use-after-free once the original instance was dropped.
    • Replaced with refcounted duplication via metacall_value_copy(self.value). Each clone now owns its own metacall_value pointer and releases it on Drop. Clones can outlive or be dropped in any order without memory safety issues.

Scope Boundary

  • In Scope: Memory safety and lifecycle corrections for MetaCallException and its integration tests.
  • Out of Scope:
    • Raw pointer API signature hardening (unsafe fn new_raw) across all wrappers (reserved for subsequent PRs).
    • Unrelated wrapper fixes (MetaCallClass, MetaCallObject, MetaCallFunction, MetaCallThrowable, MetaCallFuture, MetaCallPointer).

Verification & Testing

Runtime verification was executed inside a containerized Debian environment based on metacall/core:dev under x86_64 architecture matching MetaCall CI:

  • Build: cargo build --verbose passed cleanly with 0 errors.
  • Linter & Formatting:
    • cargo clippy -- -D warnings: Passed (0 warnings).
    • cargo fmt --check: Passed cleanly.
  • Targeted Integration Test:
    • cargo test --test metacall_exception_test -- --test-threads=1 --nocapture passed.
    • Regression tests verify:
      • Constructor and immediate drop (test_exception_new_and_drop)
      • Clone outliving the original instance (test_exception_clone_outlives_original)
      • Original outliving a clone instance (test_exception_original_outlives_clone)
      • Repeated cloning and bulk destruction (test_exception_repeated_cloning)
      • Node.js script exception load and roundtrip (test_exception_nodejs_load)
    • All tests run sequentially under a single initialize() RAII runtime guard.
  • Full Rust Port Suite:
    • cargo test -- --test-threads=1 passed: 6/6 test suites passed (100% pass rate).
  • Valgrind Memcheck:
    • Executed valgrind --leak-check=full against the compiled test binary.
    • Result: Zero errors, zero invalid reads/writes, zero bad-frees, and zero leaks originating from MetaCallException or its C lifecycle routines.

This branch has not been deployed

No deployments
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.

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

1 participant