fix (libzkp): free Rust CString by from_raw (potential memory leak) (…

…#539) * Free Rust CString by `from_raw`. * Update params version.
scroll-tech · Oct 25, 2023 · adde9ce · adde9ce
1 parent fcb3e17
commit adde9ce
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 6 deletions.
diff --git a/params/version.go b/params/version.go
@@ -24,7 +24,7 @@ import (
 const (
 	VersionMajor = 5         // Major version component of the current release
 	VersionMinor = 0         // Minor version component of the current release
-	VersionPatch = 1         // Patch version component of the current release
+	VersionPatch = 2         // Patch version component of the current release
 	VersionMeta  = "mainnet" // Version metadata to append to the version string
 )
 

diff --git a/rollup/circuitcapacitychecker/impl.go b/rollup/circuitcapacitychecker/impl.go
@@ -79,7 +79,7 @@ func (ccc *CircuitCapacityChecker) ApplyTransaction(traces *types.BlockTrace) (*
 	log.Debug("start to check circuit capacity for tx", "id", ccc.ID, "TxHash", traces.Transactions[0].TxHash)
 	rawResult := C.apply_tx(C.uint64_t(ccc.ID), tracesStr)
 	defer func() {
-		C.free(unsafe.Pointer(rawResult))
+		C.free_c_chars(rawResult)
 	}()
 	log.Debug("check circuit capacity for tx done", "id", ccc.ID, "TxHash", traces.Transactions[0].TxHash)
 
@@ -125,7 +125,7 @@ func (ccc *CircuitCapacityChecker) ApplyBlock(traces *types.BlockTrace) (*types.
 	log.Debug("start to check circuit capacity for block", "id", ccc.ID, "blockNumber", traces.Header.Number, "blockHash", traces.Header.Hash())
 	rawResult := C.apply_block(C.uint64_t(ccc.ID), tracesStr)
 	defer func() {
-		C.free(unsafe.Pointer(rawResult))
+		C.free_c_chars(rawResult)
 	}()
 	log.Debug("check circuit capacity for block done", "id", ccc.ID, "blockNumber", traces.Header.Number, "blockHash", traces.Header.Hash())
 
@@ -157,7 +157,7 @@ func (ccc *CircuitCapacityChecker) CheckTxNum(expected int) (bool, uint64, error
 	log.Debug("ccc get_tx_num start", "id", ccc.ID)
 	rawResult := C.get_tx_num(C.uint64_t(ccc.ID))
 	defer func() {
-		C.free(unsafe.Pointer(rawResult))
+		C.free_c_chars(rawResult)
 	}()
 	log.Debug("ccc get_tx_num end", "id", ccc.ID)
 
@@ -180,7 +180,7 @@ func (ccc *CircuitCapacityChecker) SetLightMode(lightMode bool) error {
 	log.Debug("ccc set_light_mode start", "id", ccc.ID)
 	rawResult := C.set_light_mode(C.uint64_t(ccc.ID), C.bool(lightMode))
 	defer func() {
-		C.free(unsafe.Pointer(rawResult))
+		C.free_c_chars(rawResult)
 	}()
 	log.Debug("ccc set_light_mode end", "id", ccc.ID)
 

diff --git a/rollup/circuitcapacitychecker/libzkp/libzkp.h b/rollup/circuitcapacitychecker/libzkp/libzkp.h
@@ -8,3 +8,4 @@ char* apply_tx(uint64_t id, char *tx_traces);
 char* apply_block(uint64_t id, char *block_trace);
 char* get_tx_num(uint64_t id);
 char* set_light_mode(uint64_t id, bool light_mode);
+void free_c_chars(char* ptr);
diff --git a/rollup/circuitcapacitychecker/libzkp/src/lib.rs b/rollup/circuitcapacitychecker/libzkp/src/lib.rs
@@ -259,11 +259,22 @@ pub mod checker {
     }
 }
 
-pub(crate) mod utils {
+pub mod utils {
     use std::ffi::{CStr, CString};
     use std::os::raw::c_char;
     use std::str::Utf8Error;
 
+    /// # Safety
+    #[no_mangle]
+    pub unsafe extern "C" fn free_c_chars(ptr: *mut c_char) {
+        if ptr.is_null() {
+            log::warn!("Try to free an empty pointer!");
+            return;
+        }
+
+        let _ = CString::from_raw(ptr);
+    }
+
     #[allow(dead_code)]
     pub(crate) fn c_char_to_str(c: *const c_char) -> Result<&'static str, Utf8Error> {
         let cstr = unsafe { CStr::from_ptr(c) };