Skip to content

Commit

Permalink
Don't leak when overwriting ex data
Browse files Browse the repository at this point in the history
  • Loading branch information
sfackler committed Nov 20, 2023
1 parent 2d9458e commit e839496
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 11 deletions.
42 changes: 32 additions & 10 deletions openssl/src/ssl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1572,16 +1572,34 @@ impl SslContextBuilder {
///
/// This can be used to provide data to callbacks registered with the context. Use the
/// `SslContext::new_ex_index` method to create an `Index`.
// FIXME should return a result
#[corresponds(SSL_CTX_set_ex_data)]
pub fn set_ex_data<T>(&mut self, index: Index<SslContext, T>, data: T) {
self.set_ex_data_inner(index, data);
}

fn set_ex_data_inner<T>(&mut self, index: Index<SslContext, T>, data: T) -> *mut c_void {
match self.ex_data_mut(index) {
Some(v) => {
*v = data;
(v as *mut T).cast()
}
_ => unsafe {
let data = Box::into_raw(Box::new(data)) as *mut c_void;
ffi::SSL_CTX_set_ex_data(self.as_ptr(), index.as_raw(), data);
data
},
}
}

fn ex_data_mut<T>(&mut self, index: Index<SslContext, T>) -> Option<&mut T> {
unsafe {
let data = Box::into_raw(Box::new(data)) as *mut c_void;
ffi::SSL_CTX_set_ex_data(self.as_ptr(), index.as_raw(), data);
data
let data = ffi::SSL_CTX_get_ex_data(self.as_ptr(), index.as_raw());
if data.is_null() {
None
} else {
Some(&mut *data.cast())
}
}
}

Expand Down Expand Up @@ -2965,15 +2983,19 @@ impl SslRef {
///
/// This can be used to provide data to callbacks registered with the context. Use the
/// `Ssl::new_ex_index` method to create an `Index`.
// FIXME should return a result
#[corresponds(SSL_set_ex_data)]
pub fn set_ex_data<T>(&mut self, index: Index<Ssl, T>, data: T) {
unsafe {
let data = Box::new(data);
ffi::SSL_set_ex_data(
self.as_ptr(),
index.as_raw(),
Box::into_raw(data) as *mut c_void,
);
match self.ex_data_mut(index) {
Some(v) => *v = data,
None => unsafe {
let data = Box::new(data);
ffi::SSL_set_ex_data(
self.as_ptr(),
index.as_raw(),
Box::into_raw(data) as *mut c_void,
);
},
}
}

Expand Down
49 changes: 48 additions & 1 deletion openssl/src/ssl/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::net::UdpSocket;
use std::net::{SocketAddr, TcpListener, TcpStream};
use std::path::Path;
use std::process::{Child, ChildStdin, Command, Stdio};
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
use std::thread;
use std::time::Duration;

Expand Down Expand Up @@ -1638,3 +1638,50 @@ fn set_security_level() {
let ssl = ssl;
assert_eq!(4, ssl.security_level());
}

#[test]
fn ssl_ctx_ex_data_leak() {
static DROPS: AtomicUsize = AtomicUsize::new(0);

struct DropTest;

impl Drop for DropTest {
fn drop(&mut self) {
DROPS.fetch_add(1, Ordering::Relaxed);
}
}

let idx = SslContext::new_ex_index().unwrap();

let mut ctx = SslContext::builder(SslMethod::tls()).unwrap();
ctx.set_ex_data(idx, DropTest);
ctx.set_ex_data(idx, DropTest);
assert_eq!(DROPS.load(Ordering::Relaxed), 1);

drop(ctx);
assert_eq!(DROPS.load(Ordering::Relaxed), 2);
}

#[test]
fn ssl_ex_data_leak() {
static DROPS: AtomicUsize = AtomicUsize::new(0);

struct DropTest;

impl Drop for DropTest {
fn drop(&mut self) {
DROPS.fetch_add(1, Ordering::Relaxed);
}
}

let idx = Ssl::new_ex_index().unwrap();

let ctx = SslContext::builder(SslMethod::tls()).unwrap().build();
let mut ssl = Ssl::new(&ctx).unwrap();
ssl.set_ex_data(idx, DropTest);
ssl.set_ex_data(idx, DropTest);
assert_eq!(DROPS.load(Ordering::Relaxed), 1);

drop(ssl);
assert_eq!(DROPS.load(Ordering::Relaxed), 2);
}

0 comments on commit e839496

Please sign in to comment.