Skip to content

Commit

Permalink
uplink: Fix passing dangling stack pointer
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
GodTamIt authored and ifraixedes committed Jan 20, 2025
1 parent ea72aad commit 66f1c62
Showing 1 changed file with 7 additions and 10 deletions.
17 changes: 7 additions & 10 deletions uplink/src/edge/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,20 +113,17 @@ impl Config {
access: &access::Grant,
opts: Option<&OptionsRegisterAccess>,
) -> Result<credentials::Gateway> {
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)
Expand Down

0 comments on commit 66f1c62

Please sign in to comment.