Skip to content

Commit d4f97a8

Browse files
userFRMclaude
andauthored
fix(ffi): reject a null stream callback at registration (#825)
A C caller can pass a null function pointer to the set-callback entries. The extern "C" fn type is non-nullable in Rust's model, so the null was stored and later invoked on the dispatcher thread when the first event arrived, dereferencing address 0 as a process-level fault the unwind boundary cannot contain. Model the parameter as Option<ThetaDataDxStreamCallback> so the null bit pattern is representable and reject it up front with a recoverable error code. The C ABI repr is unchanged (the null niche), so the header and exported symbol are identical. Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6ed69ce commit d4f97a8

1 file changed

Lines changed: 41 additions & 2 deletions

File tree

ffi/src/streaming.rs

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ pub unsafe extern "C" fn thetadatadx_client_connect_from_file(
545545
#[no_mangle]
546546
pub unsafe extern "C" fn thetadatadx_client_set_callback(
547547
handle: *const ThetaDataDxClient,
548-
callback: ThetaDataDxStreamCallback,
548+
callback: Option<ThetaDataDxStreamCallback>,
549549
ctx: *mut c_void,
550550
) -> i32 {
551551
ffi_boundary!(-1, {
@@ -555,6 +555,15 @@ pub unsafe extern "C" fn thetadatadx_client_set_callback(
555555
}
556556
// SAFETY: handle is a non-null pointer returned by the matching thetadatadx_*_new and not yet passed to thetadatadx_*_free.
557557
let handle = unsafe { &*handle };
558+
// A C caller can pass a null function pointer; modelling the
559+
// parameter as `Option` lets the null bit pattern be represented
560+
// and rejected here, instead of being stored and invoked on the
561+
// dispatcher thread where a call through address 0 would fault the
562+
// process beyond the reach of the unwind boundary.
563+
let Some(callback) = callback else {
564+
set_error("callback function pointer is null");
565+
return -1;
566+
};
558567
let cb = FfiCallback { callback, ctx };
559568
// Persist the callback BEFORE `start_streaming` so a re-entrant
560569
// call from the first delivered event sees `callback = Some`
@@ -1745,7 +1754,7 @@ where
17451754
#[no_mangle]
17461755
pub unsafe extern "C" fn thetadatadx_streaming_set_callback(
17471756
handle: *const ThetaDataDxStreamHandle,
1748-
callback: ThetaDataDxStreamCallback,
1757+
callback: Option<ThetaDataDxStreamCallback>,
17491758
ctx: *mut c_void,
17501759
) -> i32 {
17511760
ffi_boundary!(-1, {
@@ -1755,6 +1764,13 @@ pub unsafe extern "C" fn thetadatadx_streaming_set_callback(
17551764
}
17561765
// SAFETY: handle is a non-null pointer returned by the matching thetadatadx_*_new and not yet passed to thetadatadx_*_free.
17571766
let handle = unsafe { &*handle };
1767+
// A C caller can pass a null function pointer; modelling the
1768+
// parameter as `Option` lets the null bit pattern be represented
1769+
// and rejected before the dispatcher thread would invoke it.
1770+
let Some(callback) = callback else {
1771+
set_error("callback function pointer is null");
1772+
return -1;
1773+
};
17581774
// Serialise concurrent installs: `dispatcher` mutex prevents two
17591775
// racing callers from each publishing a client into `handle.inner`
17601776
// and orphaning one another's dispatcher.
@@ -2853,3 +2869,26 @@ mod panic_isolation_tests {
28532869
);
28542870
}
28552871
}
2872+
2873+
#[cfg(test)]
2874+
mod null_callback_guard_tests {
2875+
use std::ffi::c_void;
2876+
2877+
use super::{ThetaDataDxStreamCallback, ThetaDataDxStreamEvent};
2878+
2879+
extern "C" fn noop(_event: *const ThetaDataDxStreamEvent, _ctx: *mut c_void) {}
2880+
2881+
#[test]
2882+
fn null_callback_is_the_none_niche_the_guard_rejects() {
2883+
// A C caller passing a null function pointer arrives as the `None`
2884+
// niche of `Option<ThetaDataDxStreamCallback>`; both set_callback
2885+
// entries reject that before constructing an `FfiCallback`. A real
2886+
// pointer is `Some` and proceeds. This pins the representation the
2887+
// guards depend on so the parameter type cannot silently revert to
2888+
// the non-nullable `extern "C" fn`.
2889+
let null_cb: Option<ThetaDataDxStreamCallback> = None;
2890+
assert!(null_cb.is_none());
2891+
let real_cb: Option<ThetaDataDxStreamCallback> = Some(noop);
2892+
assert!(real_cb.is_some());
2893+
}
2894+
}

0 commit comments

Comments
 (0)