Skip to content

Commit

Permalink
Correctly(?) handle panics in wggors get_config
Browse files Browse the repository at this point in the history
  • Loading branch information
hulthe committed Jun 4, 2024
1 parent f2e74aa commit 9036306
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 20 deletions.
41 changes: 21 additions & 20 deletions wireguard-go-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,26 +100,27 @@ impl Tunnel {
return None;
}

// contain any cast of ptr->ref within dedicated blocks to prevent accidents
let config_len: usize;
let t: T;
{
// SAFETY: we checked for null, and wgGetConfig promises that this is a valid cstr
let config = unsafe { CStr::from_ptr(ptr) };
config_len = config.to_bytes().len();
t = f(config);
}

{
// SAFETY:
// we checked for null, and wgGetConfig promises that this is a valid cstr.
// config_len comes from the CStr above, so it should be good.
let config_bytes = unsafe { slice::from_raw_parts_mut(ptr, config_len) };
config_bytes.zeroize();
}

// SAFETY: the pointer was created by wgGetConfig, and we are no longer using it.
unsafe { ffi::wgFreePtr(ptr.cast()) };
// SAFETY: we checked for null, and wgGetConfig promises that this is a valid cstr
let config = unsafe { CStr::from_ptr(ptr) };
let config_len = config.to_bytes().len();

// execute cleanup code on Drop to make sure that it happens even if `f` panics
let on_drop = OnDrop::new(|| {
{
// SAFETY:
// we checked for null, and wgGetConfig promises that this is a valid cstr.
// config_len comes from the CStr above, so it should be good.
let config_bytes = unsafe { slice::from_raw_parts_mut(ptr, config_len) };
config_bytes.zeroize();
}

// SAFETY: the pointer was created by wgGetConfig, and we are no longer using it.
unsafe { ffi::wgFreePtr(ptr.cast()) };
});

let t = f(config);
let _ = config;
drop(on_drop);

Some(t)
}
Expand Down
15 changes: 15 additions & 0 deletions wireguard-go-rs/src/util.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
pub struct OnDrop<F: FnOnce()>(Option<F>);

impl<F: FnOnce()> OnDrop<F> {
pub fn new(f: F) -> Self {
OnDrop(Some(f))
}
}

impl<F: FnOnce()> Drop for OnDrop<F> {
fn drop(&mut self) {
if let Some(f) = self.0.take() {
f()
}
}
}

0 comments on commit 9036306

Please sign in to comment.