Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 1a09dc8

Browse filesBrowse files
authored
Merge pull request #2102 from sfackler/ex-leak
Don't leak when overwriting ex data
2 parents f456b60 + b0a1da5 commit 1a09dc8
Copy full SHA for 1a09dc8

File tree

3 files changed

+82
-11
lines changed
Filter options

3 files changed

+82
-11
lines changed

‎openssl/src/cipher_ctx.rs

Copy file name to clipboardExpand all lines: openssl/src/cipher_ctx.rs
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,7 @@ impl CipherCtxRef {
582582
/// buffer size control is maintained by the caller.
583583
///
584584
/// # Safety
585+
///
585586
/// The caller is expected to provide `output` buffer
586587
/// large enough to contain correct number of bytes. For streaming
587588
/// ciphers the output buffer size should be at least as big as
@@ -695,6 +696,7 @@ impl CipherCtxRef {
695696
/// the output buffer size check removed.
696697
///
697698
/// # Safety
699+
///
698700
/// The caller is expected to provide `output` buffer
699701
/// large enough to contain correct number of bytes. For streaming
700702
/// ciphers the output buffer can be empty, for block ciphers the

‎openssl/src/ssl/mod.rs

Copy file name to clipboardExpand all lines: openssl/src/ssl/mod.rs
+32-10Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,16 +1572,34 @@ impl SslContextBuilder {
15721572
///
15731573
/// This can be used to provide data to callbacks registered with the context. Use the
15741574
/// `SslContext::new_ex_index` method to create an `Index`.
1575+
// FIXME should return a result
15751576
#[corresponds(SSL_CTX_set_ex_data)]
15761577
pub fn set_ex_data<T>(&mut self, index: Index<SslContext, T>, data: T) {
15771578
self.set_ex_data_inner(index, data);
15781579
}
15791580

15801581
fn set_ex_data_inner<T>(&mut self, index: Index<SslContext, T>, data: T) -> *mut c_void {
1582+
match self.ex_data_mut(index) {
1583+
Some(v) => {
1584+
*v = data;
1585+
(v as *mut T).cast()
1586+
}
1587+
_ => unsafe {
1588+
let data = Box::into_raw(Box::new(data)) as *mut c_void;
1589+
ffi::SSL_CTX_set_ex_data(self.as_ptr(), index.as_raw(), data);
1590+
data
1591+
},
1592+
}
1593+
}
1594+
1595+
fn ex_data_mut<T>(&mut self, index: Index<SslContext, T>) -> Option<&mut T> {
15811596
unsafe {
1582-
let data = Box::into_raw(Box::new(data)) as *mut c_void;
1583-
ffi::SSL_CTX_set_ex_data(self.as_ptr(), index.as_raw(), data);
1584-
data
1597+
let data = ffi::SSL_CTX_get_ex_data(self.as_ptr(), index.as_raw());
1598+
if data.is_null() {
1599+
None
1600+
} else {
1601+
Some(&mut *data.cast())
1602+
}
15851603
}
15861604
}
15871605

@@ -2965,15 +2983,19 @@ impl SslRef {
29652983
///
29662984
/// This can be used to provide data to callbacks registered with the context. Use the
29672985
/// `Ssl::new_ex_index` method to create an `Index`.
2986+
// FIXME should return a result
29682987
#[corresponds(SSL_set_ex_data)]
29692988
pub fn set_ex_data<T>(&mut self, index: Index<Ssl, T>, data: T) {
2970-
unsafe {
2971-
let data = Box::new(data);
2972-
ffi::SSL_set_ex_data(
2973-
self.as_ptr(),
2974-
index.as_raw(),
2975-
Box::into_raw(data) as *mut c_void,
2976-
);
2989+
match self.ex_data_mut(index) {
2990+
Some(v) => *v = data,
2991+
None => unsafe {
2992+
let data = Box::new(data);
2993+
ffi::SSL_set_ex_data(
2994+
self.as_ptr(),
2995+
index.as_raw(),
2996+
Box::into_raw(data) as *mut c_void,
2997+
);
2998+
},
29772999
}
29783000
}
29793001

‎openssl/src/ssl/test/mod.rs

Copy file name to clipboardExpand all lines: openssl/src/ssl/test/mod.rs
+48-1Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::net::UdpSocket;
1010
use std::net::{SocketAddr, TcpListener, TcpStream};
1111
use std::path::Path;
1212
use std::process::{Child, ChildStdin, Command, Stdio};
13-
use std::sync::atomic::{AtomicBool, Ordering};
13+
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
1414
use std::thread;
1515
use std::time::Duration;
1616

@@ -1638,3 +1638,50 @@ fn set_security_level() {
16381638
let ssl = ssl;
16391639
assert_eq!(4, ssl.security_level());
16401640
}
1641+
1642+
#[test]
1643+
fn ssl_ctx_ex_data_leak() {
1644+
static DROPS: AtomicUsize = AtomicUsize::new(0);
1645+
1646+
struct DropTest;
1647+
1648+
impl Drop for DropTest {
1649+
fn drop(&mut self) {
1650+
DROPS.fetch_add(1, Ordering::Relaxed);
1651+
}
1652+
}
1653+
1654+
let idx = SslContext::new_ex_index().unwrap();
1655+
1656+
let mut ctx = SslContext::builder(SslMethod::tls()).unwrap();
1657+
ctx.set_ex_data(idx, DropTest);
1658+
ctx.set_ex_data(idx, DropTest);
1659+
assert_eq!(DROPS.load(Ordering::Relaxed), 1);
1660+
1661+
drop(ctx);
1662+
assert_eq!(DROPS.load(Ordering::Relaxed), 2);
1663+
}
1664+
1665+
#[test]
1666+
fn ssl_ex_data_leak() {
1667+
static DROPS: AtomicUsize = AtomicUsize::new(0);
1668+
1669+
struct DropTest;
1670+
1671+
impl Drop for DropTest {
1672+
fn drop(&mut self) {
1673+
DROPS.fetch_add(1, Ordering::Relaxed);
1674+
}
1675+
}
1676+
1677+
let idx = Ssl::new_ex_index().unwrap();
1678+
1679+
let ctx = SslContext::builder(SslMethod::tls()).unwrap().build();
1680+
let mut ssl = Ssl::new(&ctx).unwrap();
1681+
ssl.set_ex_data(idx, DropTest);
1682+
ssl.set_ex_data(idx, DropTest);
1683+
assert_eq!(DROPS.load(Ordering::Relaxed), 1);
1684+
1685+
drop(ssl);
1686+
assert_eq!(DROPS.load(Ordering::Relaxed), 2);
1687+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.