From 66f1c6278aabf9b5de78b3b7ab1b6bdb67a0f47c Mon Sep 17 00:00:00 2001 From: Christopher Tam Date: Sun, 19 Jan 2025 21:00:25 -0500 Subject: [PATCH] uplink: Fix passing dangling stack pointer There were 2 bugs in the `register_gateway_access()` code: 1. Most prominently, the function was constructing and passing a dangling pointer to a temporary `EdgeRegisterAccessOptions` to the FFI. In fact, that variable doesn't even live to the point that the FFI is made. While this occurs on the stack and may not always be overwritten, the compiler and program is free to reclaim and reuse that stack space for other things -- it's technically UB. This code is somewhat lucky that this doesn't occur very often because the struct is very small, on the stack, and a boolean. 2. The function was also casting the `*const EdgeRegisterAccessOptions` pointer as mutable in the unsafe block, which violates Rust's aliasing guarantees and can potentially lead to undefined behavior. --- uplink/src/edge/config.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/uplink/src/edge/config.rs b/uplink/src/edge/config.rs index 4882ae7..e39c47b 100644 --- a/uplink/src/edge/config.rs +++ b/uplink/src/edge/config.rs @@ -113,20 +113,17 @@ impl Config { access: &access::Grant, opts: Option<&OptionsRegisterAccess>, ) -> Result { - let uc_opts = if let Some(o) = opts { - &o.as_ffi_options_register_access() as *const ulksys::EdgeRegisterAccessOptions - } else { - ptr::null() - }; + // N.B.: Be sure to bind the options to a variable that lives long enough to be passed to + // the FFI. + let mut uc_opts = opts.map(|o| o.as_ffi_options_register_access()); + let uc_opts_ptr = uc_opts.as_mut().map_or(ptr::null_mut(), |o| { + o as *mut ulksys::EdgeRegisterAccessOptions + }); // SAFETY: we trust the FFI is safe creating an instance of its own types and rely in our // implemented FFI methods to return valid FFI values with correct lifetimes. let uc_res = unsafe { - ulksys::edge_register_access( - self.inner, - access.as_ffi_access(), - uc_opts as *mut ulksys::EdgeRegisterAccessOptions, - ) + ulksys::edge_register_access(self.inner, access.as_ffi_access(), uc_opts_ptr) }; credentials::Gateway::from_ffi_credentials_result(uc_res)