Unsoundness in drop_c_string

[lwz23]

Description:
The drop_c_string function is unsound because it assumes that the ptr passed to it is exclusively owned and was originally obtained through CString::into_raw. However, this assumption is not enforced in the function signature, meaning safe Rust code can inadvertently pass a pointer that violates this requirement. This can lead to undefined behavior such as double-free or heap corruption, as demonstrated in the provided PoC.

j4rs/rust/src/utils.rs

Line 44 in 18a3ac4

pub fn drop_c_string(ptr: *mut c_char) {

pub fn drop_c_string(ptr: *mut c_char) {
    let _ = unsafe { CString::from_raw(ptr) };
}

PoC:

use std::ffi::{CString};
use std::os::raw::c_char;

pub fn drop_c_string(ptr: *mut c_char) {
    let _ = unsafe { CString::from_raw(ptr) };
}

fn main() {
    // Create a CString
    let original = CString::new("Hello, world!").unwrap();

    // Obtain a raw pointer to the CString's data
    let ptr = original.as_ptr();

    // Convert the immutable pointer to a mutable pointer
    // Note: This is safe in Rust, but does not transfer ownership of the memory.
    let mut_ptr = ptr as *mut c_char;

    // Call drop_c_string with the raw pointer
    // This will lead to undefined behavior because the memory is still managed by `original`
    drop_c_string(mut_ptr);

    // At this point, `original` still believes it owns the memory,
    // but it has already been freed by drop_c_string.
    // When `original` goes out of scope, it will attempt to free the memory again, causing a double-free.
}

Result:

PS C:\Users\ROG\Desktop\cfg_test> cargo run
   Compiling cfg_test v0.1.0 (C:\Users\ROG\Desktop\cfg_test)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.24s
     Running `target\debug\cfg_test.exe`
error: process didn't exit successfully: `target\debug\cfg_test.exe` (exit code: 0xc0000374, STATUS_HEAP_CORRUPTION)

Root Cause
The issue arises because:
CString::from_raw is used in drop_c_string to take ownership of the memory pointed to by ptr and subsequently free it.
However, in the PoC, the raw pointer ptr is obtained from CString::as_ptr(), which does not transfer ownership of the memory. The memory is still managed by the original CString instance, original.
When drop_c_string is called, it incorrectly frees the memory. Later, when original goes out of scope, it also attempts to free the same memory, resulting in a double-free and heap corruption.

Why This Is Unsound
In safe Rust, it is possible to pass a pointer to drop_c_string that does not meet the assumptions required by CString::from_raw. This makes drop_c_string unsound because it allows safe Rust code to invoke undefined behavior. The function signature does not indicate that the caller must ensure ownership of the pointer, leading to incorrect assumptions about its safety.
Impact
This bug demonstrates that drop_c_string is unsound and can lead to undefined behavior in safe Rust code. Fixing this issue will ensure that safe Rust code cannot trigger undefined behavior through improper use of the function.

[astonbitecode]

Similar to #138 , I marked the drop_c_string as unsafe and made clear that the function is not exposed publicly. All required validations and correctness are enforced by the using code.

Please, if you find some path of the code that indeed leads to UB, reopen this.

Thanks for the report!