Skip to content

Fix use-after-free from JavaScript #3272

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Fizzixnerd
Copy link
Contributor

bindings for Lagrange commitment operations.

Changes:

  • Added lagrange_commitments_whole_domain_take: Safely transfers
    ownership of the pointer data, preventing memory leaks
  • Added lagrange_commitments_whole_domain_free: Explicitly frees
    allocated memory without returning data
  • Fixed read_from_ptr: Now properly handles memory to avoid
    use-after-free issues
  • Added documentation: Marked the old read_from_ptr as deprecated in
    favor of the new take function

Impact:

These changes support o1js memory leak fixes by providing safe ownership
transfer mechanisms for Lagrange commitments, which are used heavily in
recursive proof generation where memory leaks were most problematic.

Merge branch 'master' into dw/bumping-up-wasm-bindgen and add memory leak fixes
for Lagrange commitment operations that prevent memory accumulation during proof
generation processes.
@Fizzixnerd Fizzixnerd requested a review from dannywillems June 26, 2025 20:14
@Fizzixnerd
Copy link
Contributor Author

More background.

The Original Problem

The original lagrange_commitments_whole_domain_read_from_ptr function had
a use-after-free vulnerability:

// Original buggy implementation
pub unsafe fn lagrange_commitments_whole_domain_read_from_ptr(
ptr: *mut WasmVector,
) -> WasmVector {
let b = unsafe { Box::from_raw(ptr) };
*b // This moves the value out and drops the Box!
}

The bug: When you dereference *b, it moves the value out of the Box AND
automatically drops the Box, which frees the memory at ptr. But the
caller still has the pointer and might try to use it again, causing
use-after-free.

The Fix

The PR adds two new safe functions:

  1. lagrange_commitments_whole_domain_take - Explicitly transfers
    ownership:
    pub unsafe fn lagrange_commitments_whole_domain_take(
    ptr: *mut WasmVector,
    ) -> WasmVector {
    let b = unsafe { Box::from_raw(ptr) };
    *b // Same code, but with clear ownership semantics
    }

  2. lagrange_commitments_whole_domain_free - Just frees without returning:
    pub unsafe fn lagrange_commitments_whole_domain_free(
    ptr: *mut WasmVector,
    ) {
    let _ = unsafe { Box::from_raw(ptr) }; // Drops immediately
    }

Why This Matters

In o1js, the JavaScript side was calling read_from_ptr multiple times on
the same pointer during recursive proof generation, expecting the memory
to stay valid. The new take function makes it clear that the pointer
becomes invalid after use, forcing the JavaScript side to properly manage
memory lifecycle.

@Fizzixnerd Fizzixnerd self-assigned this Jun 26, 2025
@Fizzixnerd Fizzixnerd added the bug Something isn't working label Jun 26, 2025
@Fizzixnerd Fizzixnerd changed the title Fix proving memory leak by 20-25% of o1js Fix use-after-free from JavaScript Jun 27, 2025
@Fizzixnerd Fizzixnerd marked this pull request as draft June 27, 2025 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant