Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a utility function to ensure we never have an issue with 0-length slices from pointers again #2268

Merged
merged 1 commit into from
Jul 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
disallowed-methods = [
{ path = "std::slice::from_raw_parts", reason = "use util::from_raw_parts instead" },
{ path = "std::slice::from_raw_parts_mut", reason = "use util::from_raw_parts_mut instead" },
]
11 changes: 5 additions & 6 deletions openssl/src/asn1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use std::convert::TryInto;
use std::ffi::CString;
use std::fmt;
use std::ptr;
use std::slice;
use std::str;

use crate::bio::MemBio;
Expand All @@ -41,7 +40,7 @@ use crate::error::ErrorStack;
use crate::nid::Nid;
use crate::stack::Stackable;
use crate::string::OpensslString;
use crate::{cvt, cvt_p};
use crate::{cvt, cvt_p, util};
use openssl_macros::corresponds;

foreign_type_and_impl_send_sync! {
Expand Down Expand Up @@ -457,7 +456,7 @@ impl Asn1StringRef {
/// [`as_utf8`]: struct.Asn1String.html#method.as_utf8
#[corresponds(ASN1_STRING_get0_data)]
pub fn as_slice(&self) -> &[u8] {
unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr()), self.len()) }
unsafe { util::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr()), self.len()) }
}

/// Returns the number of bytes in the string.
Expand Down Expand Up @@ -597,7 +596,7 @@ impl Asn1BitStringRef {
/// Returns the Asn1BitString as a slice.
#[corresponds(ASN1_STRING_get0_data)]
pub fn as_slice(&self) -> &[u8] {
unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr() as *mut _), self.len()) }
unsafe { util::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr() as *mut _), self.len()) }
}

/// Returns the number of bytes in the string.
Expand Down Expand Up @@ -637,7 +636,7 @@ impl Asn1OctetStringRef {
/// Returns the octet string as an array of bytes.
#[corresponds(ASN1_STRING_get0_data)]
pub fn as_slice(&self) -> &[u8] {
unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr().cast()), self.len()) }
unsafe { util::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr().cast()), self.len()) }
}

/// Returns the number of bytes in the octet string.
Expand Down Expand Up @@ -701,7 +700,7 @@ impl Asn1Object {
pub fn as_slice(&self) -> &[u8] {
unsafe {
let len = ffi::OBJ_length(self.as_ptr());
slice::from_raw_parts(ffi::OBJ_get0_data(self.as_ptr()), len)
util::from_raw_parts(ffi::OBJ_get0_data(self.as_ptr()), len)
}
}
}
Expand Down
8 changes: 2 additions & 6 deletions openssl/src/bio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ use cfg_if::cfg_if;
use libc::c_int;
use std::marker::PhantomData;
use std::ptr;
use std::slice;

use crate::cvt_p;
use crate::error::ErrorStack;
use crate::util;

pub struct MemBioSlice<'a>(*mut ffi::BIO, PhantomData<&'a [u8]>);

Expand Down Expand Up @@ -63,11 +63,7 @@ impl MemBio {
unsafe {
let mut ptr = ptr::null_mut();
let len = ffi::BIO_get_mem_data(self.0, &mut ptr);
if len == 0 {
&[]
} else {
slice::from_raw_parts(ptr as *const _ as *const _, len as usize)
}
util::from_raw_parts(ptr as *const _ as *const _, len as usize)
}
}

Expand Down
7 changes: 3 additions & 4 deletions openssl/src/ssl/bio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ use std::io;
use std::io::prelude::*;
use std::panic::{catch_unwind, AssertUnwindSafe};
use std::ptr;
use std::slice;

use crate::cvt_p;
use crate::error::ErrorStack;
use crate::{cvt_p, util};

pub struct StreamState<S> {
pub stream: S,
Expand Down Expand Up @@ -89,7 +88,7 @@ unsafe extern "C" fn bwrite<S: Write>(bio: *mut BIO, buf: *const c_char, len: c_
BIO_clear_retry_flags(bio);

let state = state::<S>(bio);
let buf = slice::from_raw_parts(buf as *const _, len as usize);
let buf = util::from_raw_parts(buf as *const _, len as usize);

match catch_unwind(AssertUnwindSafe(|| state.stream.write(buf))) {
Ok(Ok(len)) => len as c_int,
Expand All @@ -111,7 +110,7 @@ unsafe extern "C" fn bread<S: Read>(bio: *mut BIO, buf: *mut c_char, len: c_int)
BIO_clear_retry_flags(bio);

let state = state::<S>(bio);
let buf = slice::from_raw_parts_mut(buf as *mut _, len as usize);
let buf = util::from_raw_parts_mut(buf as *mut _, len as usize);

match catch_unwind(AssertUnwindSafe(|| state.stream.read(buf))) {
Ok(Ok(len)) => len as c_int,
Expand Down
22 changes: 11 additions & 11 deletions openssl/src/ssl/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use libc::{c_int, c_uchar, c_uint, c_void};
use std::ffi::CStr;
use std::mem;
use std::ptr;
use std::slice;
#[cfg(any(ossl111, boringssl))]
use std::str;
use std::sync::Arc;
Expand All @@ -28,6 +27,7 @@ use crate::ssl::{
};
#[cfg(ossl111)]
use crate::ssl::{ClientHelloResponse, ExtensionContext};
use crate::util;
#[cfg(any(ossl111, boringssl))]
use crate::util::ForeignTypeRefExt;
#[cfg(ossl111)]
Expand Down Expand Up @@ -85,9 +85,9 @@ where
None
};
// Give the callback mutable slices into which it can write the identity and psk.
let identity_sl = slice::from_raw_parts_mut(identity as *mut u8, max_identity_len as usize);
let identity_sl = util::from_raw_parts_mut(identity as *mut u8, max_identity_len as usize);
#[allow(clippy::unnecessary_cast)]
let psk_sl = slice::from_raw_parts_mut(psk as *mut u8, max_psk_len as usize);
let psk_sl = util::from_raw_parts_mut(psk as *mut u8, max_psk_len as usize);
match (*callback)(ssl, hint, identity_sl, psk_sl) {
Ok(psk_len) => psk_len as u32,
Err(e) => {
Expand Down Expand Up @@ -126,7 +126,7 @@ where
};
// Give the callback mutable slices into which it can write the psk.
#[allow(clippy::unnecessary_cast)]
let psk_sl = slice::from_raw_parts_mut(psk as *mut u8, max_psk_len as usize);
let psk_sl = util::from_raw_parts_mut(psk as *mut u8, max_psk_len as usize);
match (*callback)(ssl, identity, psk_sl) {
Ok(psk_len) => psk_len as u32,
Err(e) => {
Expand Down Expand Up @@ -197,7 +197,7 @@ where
.ex_data(SslContext::cached_ex_index::<F>())
.expect("BUG: alpn callback missing") as *const F;
#[allow(clippy::unnecessary_cast)]
let protos = slice::from_raw_parts(inbuf as *const u8, inlen as usize);
let protos = util::from_raw_parts(inbuf as *const u8, inlen as usize);

match (*callback)(ssl, protos) {
Ok(proto) => {
Expand Down Expand Up @@ -416,7 +416,7 @@ where
.ex_data(SslContext::cached_ex_index::<F>())
.expect("BUG: get session callback missing") as *const F;
#[allow(clippy::unnecessary_cast)]
let data = slice::from_raw_parts(data as *const u8, len as usize);
let data = util::from_raw_parts(data as *const u8, len as usize);

match (*callback)(ssl, data) {
Some(session) => {
Expand Down Expand Up @@ -460,7 +460,7 @@ where
.ex_data(SslContext::cached_ex_index::<F>())
.expect("BUG: stateless cookie generate callback missing") as *const F;
#[allow(clippy::unnecessary_cast)]
let slice = slice::from_raw_parts_mut(cookie as *mut u8, ffi::SSL_COOKIE_LENGTH as usize);
let slice = util::from_raw_parts_mut(cookie as *mut u8, ffi::SSL_COOKIE_LENGTH as usize);
match (*callback)(ssl, slice) {
Ok(len) => {
*cookie_len = len as size_t;
Expand Down Expand Up @@ -488,7 +488,7 @@ where
.ex_data(SslContext::cached_ex_index::<F>())
.expect("BUG: stateless cookie verify callback missing") as *const F;
#[allow(clippy::unnecessary_cast)]
let slice = slice::from_raw_parts(cookie as *const c_uchar as *const u8, cookie_len);
let slice = util::from_raw_parts(cookie as *const c_uchar as *const u8, cookie_len);
(*callback)(ssl, slice) as c_int
}

Expand All @@ -511,7 +511,7 @@ where
// compatibility. See comments in dtls1.h.
#[allow(clippy::unnecessary_cast)]
let slice =
slice::from_raw_parts_mut(cookie as *mut u8, ffi::DTLS1_COOKIE_LENGTH as usize - 1);
util::from_raw_parts_mut(cookie as *mut u8, ffi::DTLS1_COOKIE_LENGTH as usize - 1);
match (*callback)(ssl, slice) {
Ok(len) => {
*cookie_len = len as c_uint;
Expand Down Expand Up @@ -551,7 +551,7 @@ where
.expect("BUG: cookie verify callback missing") as *const F;
#[allow(clippy::unnecessary_cast)]
let slice =
slice::from_raw_parts(cookie as *const c_uchar as *const u8, cookie_len as usize);
util::from_raw_parts(cookie as *const c_uchar as *const u8, cookie_len as usize);
(*callback)(ssl, slice) as c_int
}
}
Expand Down Expand Up @@ -663,7 +663,7 @@ where
.expect("BUG: custom ext parse callback missing") as *const F;
let ectx = ExtensionContext::from_bits_truncate(context);
#[allow(clippy::unnecessary_cast)]
let slice = slice::from_raw_parts(input as *const u8, inlen);
let slice = util::from_raw_parts(input as *const u8, inlen);
let cert = if ectx.contains(ExtensionContext::TLS1_3_CERTIFICATE) {
Some((chainidx, X509Ref::from_ptr(x)))
} else {
Expand Down
22 changes: 11 additions & 11 deletions openssl/src/ssl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ use crate::ssl::bio::BioMethod;
use crate::ssl::callbacks::*;
use crate::ssl::error::InnerError;
use crate::stack::{Stack, StackRef, Stackable};
use crate::util;
use crate::util::{ForeignTypeExt, ForeignTypeRefExt};
use crate::x509::store::{X509Store, X509StoreBuilderRef, X509StoreRef};
#[cfg(any(ossl102, boringssl, libressl261))]
Expand All @@ -101,7 +102,6 @@ use std::ops::{Deref, DerefMut};
use std::panic::resume_unwind;
use std::path::Path;
use std::ptr;
use std::slice;
use std::str;
use std::sync::{Arc, Mutex};

Expand Down Expand Up @@ -708,7 +708,7 @@ pub fn select_next_proto<'a>(server: &[u8], client: &'a [u8]) -> Option<&'a [u8]
client.len() as c_uint,
);
if r == ffi::OPENSSL_NPN_NEGOTIATED {
Some(slice::from_raw_parts(out as *const u8, outlen as usize))
Some(util::from_raw_parts(out as *const u8, outlen as usize))
} else {
None
}
Expand Down Expand Up @@ -2174,7 +2174,7 @@ impl SslSessionRef {
let mut len = 0;
let p = ffi::SSL_SESSION_get_id(self.as_ptr(), &mut len);
#[allow(clippy::unnecessary_cast)]
slice::from_raw_parts(p as *const u8, len as usize)
util::from_raw_parts(p as *const u8, len as usize)
}
}

Expand Down Expand Up @@ -2650,7 +2650,7 @@ impl SslRef {
if data.is_null() {
None
} else {
Some(slice::from_raw_parts(data, len as usize))
Some(util::from_raw_parts(data, len as usize))
}
}
}
Expand Down Expand Up @@ -2928,7 +2928,7 @@ impl SslRef {
if len < 0 {
None
} else {
Some(slice::from_raw_parts(p as *const u8, len as usize))
Some(util::from_raw_parts(p as *const u8, len as usize))
}
}
}
Expand Down Expand Up @@ -3099,7 +3099,7 @@ impl SslRef {
if len == 0 {
None
} else {
Some(slice::from_raw_parts(ptr, len))
Some(util::from_raw_parts(ptr, len))
}
}
}
Expand All @@ -3118,7 +3118,7 @@ impl SslRef {
if len == 0 {
None
} else {
Some(slice::from_raw_parts(ptr, len))
Some(util::from_raw_parts(ptr, len))
}
}
}
Expand All @@ -3137,7 +3137,7 @@ impl SslRef {
if len == 0 {
None
} else {
Some(slice::from_raw_parts(ptr, len))
Some(util::from_raw_parts(ptr, len))
}
}
}
Expand Down Expand Up @@ -3191,7 +3191,7 @@ impl SslRef {
if len == 0 {
None
} else {
Some(slice::from_raw_parts(ptr, len))
Some(util::from_raw_parts(ptr, len))
}
}
}
Expand Down Expand Up @@ -3764,7 +3764,7 @@ impl<S: Read + Write> SslStream<S> {
pub fn ssl_read(&mut self, buf: &mut [u8]) -> Result<usize, Error> {
// SAFETY: `ssl_read_uninit` does not de-initialize the buffer.
unsafe {
self.ssl_read_uninit(slice::from_raw_parts_mut(
self.ssl_read_uninit(util::from_raw_parts_mut(
buf.as_mut_ptr().cast::<MaybeUninit<u8>>(),
buf.len(),
))
Expand Down Expand Up @@ -3997,7 +3997,7 @@ impl<S: Read + Write> Read for SslStream<S> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
// SAFETY: `read_uninit` does not de-initialize the buffer
unsafe {
self.read_uninit(slice::from_raw_parts_mut(
self.read_uninit(util::from_raw_parts_mut(
buf.as_mut_ptr().cast::<MaybeUninit<u8>>(),
buf.len(),
))
Expand Down
27 changes: 26 additions & 1 deletion openssl/src/util.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::error::ErrorStack;
use crate::util;
use foreign_types::{ForeignType, ForeignTypeRef};
use libc::{c_char, c_int, c_void};
use std::any::Any;
Expand Down Expand Up @@ -49,7 +50,7 @@ where
let callback = &mut *(cb_state as *mut CallbackState<F>);

let result = panic::catch_unwind(AssertUnwindSafe(|| {
let pass_slice = slice::from_raw_parts_mut(buf as *mut u8, size as usize);
let pass_slice = util::from_raw_parts_mut(buf as *mut u8, size as usize);
callback.cb.take().unwrap()(pass_slice)
}));

Expand Down Expand Up @@ -91,3 +92,27 @@ pub trait ForeignTypeRefExt: ForeignTypeRef {
}
}
impl<FT: ForeignTypeRef> ForeignTypeRefExt for FT {}

/// The same as `slice::from_raw_parts`, except that `data` may be `NULL` if
/// `len` is 0.
pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] {
if len == 0 {
&[]
} else {
// Using this to implement the preferred API
#[allow(clippy::disallowed_methods)]
slice::from_raw_parts(data, len)
}
}

/// The same as `slice::from_raw_parts_mut`, except that `data` may be `NULL`
/// if `len` is 0.
pub unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a mut [T] {
if len == 0 {
&mut []
} else {
// Using this to implement the preferred API
#[allow(clippy::disallowed_methods)]
slice::from_raw_parts_mut(data, len)
}
}
Loading