proof-systems icon indicating copy to clipboard operation
proof-systems copied to clipboard

Fix use-after-free from JavaScript

Open Fizzixnerd opened this issue 6 months ago • 1 comments

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.

Fizzixnerd avatar Jun 26 '25 20:06 Fizzixnerd

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<WasmPolyComm>, ) -> WasmVector<WasmPolyComm> { 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<WasmPolyComm>, ) -> WasmVector<WasmPolyComm> { 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<WasmPolyComm>, ) { 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 avatar Jun 26 '25 20:06 Fizzixnerd