From 24a56032af17fc95d9c6fe2a96e96231d34cf118 Mon Sep 17 00:00:00 2001 From: Ivan FB Date: Mon, 11 May 2026 09:40:33 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20Rust=20callbacks=20dereference=20msg=20wi?= =?UTF-8?q?thout=20a=20null=20check=20(rust.nim:272,=20317).=20CStr::from?= =?UTF-8?q?=5Fptr(msg)=20is=20UB=20if=20msg=20=3D=3D=20nullptr.=20The=20C+?= =?UTF-8?q?+=20side=20guards=20with=20msg=20=3F=20...=20:=20"".=20The=20Ni?= =?UTF-8?q?m=20side=20=20=20appears=20to=20always=20pass=20a=20non-null=20?= =?UTF-8?q?pointer,=20but=20soundness=20across=20an=20FFI=20boundary=20sho?= =?UTF-8?q?uldn't=20hinge=20on=20a=20producer's=20discipline=20=E2=80=94?= =?UTF-8?q?=20the=20Rust=20receiver=20should=20null-check.=20Especially=20?= =?UTF-8?q?=20=20since=20the=20C=20ABI=20signature=20here=20is=20the=20one?= =?UTF-8?q?=20downstream=20consumers=20will=20rely=20on=20indefinitely.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- examples/nim_timer/rust_bindings/src/api.rs | 25 ++++++++++++--------- ffi/codegen/rust.nim | 25 ++++++++++++--------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/examples/nim_timer/rust_bindings/src/api.rs b/examples/nim_timer/rust_bindings/src/api.rs index 009aa4b..960431e 100644 --- a/examples/nim_timer/rust_bindings/src/api.rs +++ b/examples/nim_timer/rust_bindings/src/api.rs @@ -12,6 +12,17 @@ struct FfiCallbackResult { type Pair = Arc<(Mutex, Condvar)>; +/// Null-safe conversion from a C string returned by the Nim side. +/// The C ABI permits a null `msg` even though the current producer +/// always passes a non-null pointer; treat null as an empty string. +unsafe fn cstr_to_string(msg: *const c_char) -> String { + if msg.is_null() { + String::new() + } else { + CStr::from_ptr(msg).to_string_lossy().into_owned() + } +} + unsafe extern "C" fn on_result( ret: c_int, msg: *const c_char, @@ -21,11 +32,8 @@ unsafe extern "C" fn on_result( let pair = Arc::from_raw(user_data as *const (Mutex, Condvar)); let (lock, cvar) = &*pair; let mut state = lock.lock().unwrap(); - state.payload = Some(if ret == 0 { - Ok(CStr::from_ptr(msg).to_string_lossy().into_owned()) - } else { - Err(CStr::from_ptr(msg).to_string_lossy().into_owned()) - }); + let s = cstr_to_string(msg); + state.payload = Some(if ret == 0 { Ok(s) } else { Err(s) }); cvar.notify_one(); } @@ -60,11 +68,8 @@ unsafe extern "C" fn on_result_async( let tx = Box::from_raw( user_data as *mut tokio::sync::oneshot::Sender>, ); - let value = if ret == 0 { - Ok(CStr::from_ptr(msg).to_string_lossy().into_owned()) - } else { - Err(CStr::from_ptr(msg).to_string_lossy().into_owned()) - }; + let s = cstr_to_string(msg); + let value = if ret == 0 { Ok(s) } else { Err(s) }; let _ = tx.send(value); } diff --git a/ffi/codegen/rust.nim b/ffi/codegen/rust.nim index 28e265f..d489e41 100644 --- a/ffi/codegen/rust.nim +++ b/ffi/codegen/rust.nim @@ -258,6 +258,17 @@ proc generateApiRs*(procs: seq[FFIProcMeta], libName: string): string = lines.add("") lines.add("type Pair = Arc<(Mutex, Condvar)>;") lines.add("") + lines.add("/// Null-safe conversion from a C string returned by the Nim side.") + lines.add("/// The C ABI permits a null `msg` even though the current producer") + lines.add("/// always passes a non-null pointer; treat null as an empty string.") + lines.add("unsafe fn cstr_to_string(msg: *const c_char) -> String {") + lines.add(" if msg.is_null() {") + lines.add(" String::new()") + lines.add(" } else {") + lines.add(" CStr::from_ptr(msg).to_string_lossy().into_owned()") + lines.add(" }") + lines.add("}") + lines.add("") lines.add("unsafe extern \"C\" fn on_result(") lines.add(" ret: c_int,") lines.add(" msg: *const c_char,") @@ -267,11 +278,8 @@ proc generateApiRs*(procs: seq[FFIProcMeta], libName: string): string = lines.add(" let pair = Arc::from_raw(user_data as *const (Mutex, Condvar));") lines.add(" let (lock, cvar) = &*pair;") lines.add(" let mut state = lock.lock().unwrap();") - lines.add(" state.payload = Some(if ret == 0 {") - lines.add(" Ok(CStr::from_ptr(msg).to_string_lossy().into_owned())") - lines.add(" } else {") - lines.add(" Err(CStr::from_ptr(msg).to_string_lossy().into_owned())") - lines.add(" });") + lines.add(" let s = cstr_to_string(msg);") + lines.add(" state.payload = Some(if ret == 0 { Ok(s) } else { Err(s) });") lines.add(" cvar.notify_one();") lines.add("}") lines.add("") @@ -311,11 +319,8 @@ proc generateApiRs*(procs: seq[FFIProcMeta], libName: string): string = lines.add(" let tx = Box::from_raw(") lines.add(" user_data as *mut tokio::sync::oneshot::Sender>,") lines.add(" );") - lines.add(" let value = if ret == 0 {") - lines.add(" Ok(CStr::from_ptr(msg).to_string_lossy().into_owned())") - lines.add(" } else {") - lines.add(" Err(CStr::from_ptr(msg).to_string_lossy().into_owned())") - lines.add(" };") + lines.add(" let s = cstr_to_string(msg);") + lines.add(" let value = if ret == 0 { Ok(s) } else { Err(s) };") lines.add(" let _ = tx.send(value);") lines.add("}") lines.add("")