Safer slices

boring/src/asn1.rs

@@ -24,23 +24,23 @@
 //! use boring::asn1::Asn1Time;
 //! let tomorrow = Asn1Time::days_from_now(1);
 //! ```
-use crate::ffi;
 use foreign_types::{ForeignType, ForeignTypeRef};
 use libc::{c_int, c_long, time_t};
 use std::cmp::Ordering;
 use std::ffi::CString;
 use std::fmt;
 use std::ptr;
-use std::slice;
 use std::str;
 
 use crate::bio::MemBio;
 use crate::bn::{BigNum, BigNumRef};
 use crate::error::ErrorStack;
+use crate::ffi;
 use crate::nid::Nid;
 use crate::stack::Stackable;
 use crate::string::OpensslString;
-use crate::{cvt, cvt_p};
+use crate::try_slice;
+use crate::{cvt, cvt_n, cvt_p};
 use openssl_macros::corresponds;
 
 foreign_type_and_impl_send_sync! {
@@ -403,11 +403,7 @@ impl Asn1StringRef {
     pub fn as_utf8(&self) -> Result<OpensslString, ErrorStack> {
         unsafe {
             let mut ptr = ptr::null_mut();
-            let len = ffi::ASN1_STRING_to_UTF8(&mut ptr, self.as_ptr());
-            if len < 0 {
-                return Err(ErrorStack::get());
-            }
-
+            cvt_n(ffi::ASN1_STRING_to_UTF8(&mut ptr, self.as_ptr()))?;
             Ok(OpensslString::from_ptr(ptr.cast()))
         }
     }
@@ -421,7 +417,7 @@ impl Asn1StringRef {
     #[corresponds(ASN1_STRING_get0_data)]
     #[must_use]
     pub fn as_slice(&self) -> &[u8] {
-        unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr()), self.len()) }
+        unsafe { try_slice(ASN1_STRING_get0_data(self.as_ptr()), self.len()).unwrap_or(&[]) }
     }
 
     /// Returns the number of bytes in the string.
@@ -529,13 +525,7 @@ impl Asn1BitStringRef {
     #[corresponds(ASN1_STRING_get0_data)]
     #[must_use]
     pub fn as_slice(&self) -> &[u8] {
-        unsafe {
-            let ptr = ASN1_STRING_get0_data(self.as_ptr().cast());
-            if ptr.is_null() {
-                return &[];
-            }
-            slice::from_raw_parts(ptr, self.len())
-        }
+        unsafe { try_slice(ASN1_STRING_get0_data(self.as_ptr().cast()), self.len()).unwrap_or(&[]) }
     }
 
     /// Returns the Asn1BitString as a str, if possible.

boring/src/bio.rs

@@ -1,12 +1,11 @@
 use std::marker::PhantomData;
 use std::ptr;
-use std::slice;
 
-use crate::cvt_p;
 use crate::error::ErrorStack;
 use crate::ffi;
 use crate::ffi::BIO_new_mem_buf;
-use crate::try_int;
+use crate::{cvt, cvt_p};
+use crate::{try_int, try_slice};
 
 pub struct MemBioSlice<'a>(*mut ffi::BIO, PhantomData<&'a [u8]>);
 
@@ -54,14 +53,17 @@ impl MemBio {
         self.0
     }
 
+    /// An empty slice may indicate an error, use [`Self::try_get_buf`] instead.
     pub fn get_buf(&self) -> &[u8] {
+        self.try_get_buf().unwrap_or(&[])
+    }
+
+    pub fn try_get_buf(&self) -> Result<&[u8], ErrorStack> {
         unsafe {
-            let mut ptr = ptr::null_mut();
-            let len = ffi::BIO_get_mem_data(self.0, &mut ptr);
-            if ptr.is_null() || len < 0 {
-                return &[];
-            }
-            slice::from_raw_parts(ptr.cast_const().cast(), len as usize)
+            let mut ptr: *const u8 = ptr::null_mut();
+            let mut len = 0;
+            cvt(ffi::BIO_mem_contents(self.0, &mut ptr, &mut len))?;
+            try_slice(ptr, len).ok_or_else(|| ErrorStack::internal_error_str("invalid slice"))
         }
     }
 }

boring/src/lib.rs

@@ -170,11 +170,11 @@ fn cvt_0(r: usize) -> Result<(), ErrorStack> {
     }
 }
 
-fn cvt_0i(r: c_int) -> Result<c_int, ErrorStack> {
+fn cvt_0i(r: c_int) -> Result<(), ErrorStack> {
     if r == 0 {
         Err(ErrorStack::get())
     } else {
-        Ok(r)
+        Ok(())
     }
 }
 
@@ -201,6 +201,48 @@ fn cvt_n(r: c_int) -> Result<c_int, ErrorStack> {
     }
 }
 
+unsafe fn try_slice<'a, T: Sized + 'static, L: TryInto<usize> + Copy + 'static>(
+    ptr: *const T,
+    len_in_items: L,
+) -> Option<&'a [T]> {
+    safe_slice_size::<T, L>(len_in_items)
+        .filter(|_| !ptr.is_null())
+        .map(|len| {
+            // C/C++ may assume the pointer can be anything if it's not dereferenced, which isn't true in Rust
+            if len > 0 {
+                std::slice::from_raw_parts(ptr, len)
+            } else {
+                &[]
+            }
+        })
+}
+
+unsafe fn try_slice_mut<'a, T: Sized + 'static, L: TryInto<usize> + Copy + 'static>(
+    ptr: *mut T,
+    len_in_items: L,
+) -> Result<&'a mut [T], ErrorStack> {
+    safe_slice_size::<T, L>(len_in_items)
+        .filter(|_| !ptr.is_null())
+        .map(|len| {
+            if len > 0 {
+                std::slice::from_raw_parts_mut(ptr, len)
+            } else {
+                &mut []
+            }
+        })
+        .ok_or_else(|| ErrorStack::internal_error_str("invalid slice"))
+}
+
+fn safe_slice_size<T: Sized, L: TryInto<usize>>(len_in_items: L) -> Option<usize> {
+    let len = len_in_items.try_into().ok()?;
+    // it's UB to have larger allocation size in Rust
+    if len.checked_mul(size_of::<T>())? < isize::MAX as usize {
+        Some(len)
+    } else {
+        None
+    }
+}
+
 fn try_int<F, T>(from: F) -> Result<T, ErrorStack>
 where
     F: TryInto<T> + Send + Sync + Copy + 'static,

boring/src/pkey.rs

@@ -235,7 +235,7 @@ where
     pub fn raw_public_key_len(&self) -> Result<usize, ErrorStack> {
         unsafe {
             let mut size = 0;
-            _ = cvt_0i(ffi::EVP_PKEY_get_raw_public_key(
+            cvt_0i(ffi::EVP_PKEY_get_raw_public_key(
                 self.as_ptr(),
                 std::ptr::null_mut(),
                 &mut size,
@@ -251,7 +251,7 @@ where
     pub fn raw_public_key<'a>(&self, out: &'a mut [u8]) -> Result<&'a [u8], ErrorStack> {
         unsafe {
             let mut size = out.len();
-            _ = cvt_0i(ffi::EVP_PKEY_get_raw_public_key(
+            cvt_0i(ffi::EVP_PKEY_get_raw_public_key(
                 self.as_ptr(),
                 out.as_mut_ptr(),
                 &mut size,
@@ -303,7 +303,7 @@ where
     pub fn raw_private_key_len(&self) -> Result<usize, ErrorStack> {
         unsafe {
             let mut size = 0;
-            _ = cvt_0i(ffi::EVP_PKEY_get_raw_private_key(
+            cvt_0i(ffi::EVP_PKEY_get_raw_private_key(
                 self.as_ptr(),
                 std::ptr::null_mut(),
                 &mut size,
@@ -319,7 +319,7 @@ where
     pub fn raw_private_key<'a>(&self, out: &'a mut [u8]) -> Result<&'a [u8], ErrorStack> {
         unsafe {
             let mut size = out.len();
-            _ = cvt_0i(ffi::EVP_PKEY_get_raw_private_key(
+            cvt_0i(ffi::EVP_PKEY_get_raw_private_key(
                 self.as_ptr(),
                 out.as_mut_ptr(),
                 &mut size,

boring/src/ssl/bio.rs

@@ -8,10 +8,10 @@ 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, try_int};
+use crate::{try_slice, try_slice_mut};
 
 pub struct StreamState<S> {
     pub stream: S,
@@ -106,7 +106,9 @@ unsafe extern "C" fn bwrite<S: Write>(bio: *mut BIO, buf: *const c_char, len: c_
     };
 
     let state = state::<S>(bio);
-    let buf = slice::from_raw_parts(buf.cast(), len);
+    let Some(buf) = try_slice(buf.cast(), len) else {
+        return -1;
+    };
 
     match catch_unwind(AssertUnwindSafe(|| state.stream.write(buf))) {
         Ok(Ok(len)) => len as c_int,
@@ -127,15 +129,20 @@ unsafe extern "C" fn bwrite<S: Write>(bio: *mut BIO, buf: *const c_char, len: c_
 unsafe extern "C" fn bread<S: Read>(bio: *mut BIO, buf: *mut c_char, len: c_int) -> c_int {
     BIO_clear_retry_flags(bio);
 
-    let Ok(len) = usize::try_from(len) else {
-        return -1;
-    };
-
     let state = state::<S>(bio);
-    let buf = slice::from_raw_parts_mut(buf.cast(), len);
 
-    match catch_unwind(AssertUnwindSafe(|| state.stream.read(buf))) {
-        Ok(Ok(len)) => len as c_int,
+    let buf = try_int(len)
+        .and_then(|len: usize| unsafe { try_slice_mut(buf.cast(), len) })
+        .map_err(io::Error::other);
+
+    let res = catch_unwind(AssertUnwindSafe(|| {
+        state
+            .stream
+            .read(buf?)
+            .and_then(|len| c_int::try_from(len).map_err(io::Error::other))
+    }));
+    match res {
+        Ok(Ok(len)) => len,
         Ok(Err(err)) => {
             if retriable_error(&err) {
                 BIO_set_retry_read(bio);

boring/src/ssl/callbacks.rs

@@ -12,6 +12,7 @@ use crate::hmac::HmacCtxRef;
 use crate::ssl::TicketKeyCallbackResult;
 use crate::symm::CipherCtxRef;
 use crate::x509::{X509StoreContext, X509StoreContextRef};
+use crate::{try_slice, try_slice_mut};
 use foreign_types::ForeignType;
 use foreign_types::ForeignTypeRef;
 use libc::{c_char, c_int, c_uchar, c_uint, c_void};
@@ -666,7 +667,9 @@ where
         .ex_data(SslContext::cached_ex_index::<C>())
         .expect("BUG: certificate compression missed");
 
-    let input_slice = unsafe { std::slice::from_raw_parts(input, input_len) };
+    let Some(input_slice) = (unsafe { try_slice(input, input_len) }) else {
+        return 0;
+    };
     let mut writer = CryptoByteBuilder::from_ptr(out);
     if compressor.compress(input_slice, &mut writer).is_err() {
         return 0;
@@ -755,7 +758,7 @@ impl<'a> CryptoBufferBuilder<'a> {
         let buffer = unsafe { crate::cvt_p(ffi::CRYPTO_BUFFER_alloc(&mut data, capacity))? };
         Ok(CryptoBufferBuilder {
             buffer,
-            cursor: std::io::Cursor::new(unsafe { std::slice::from_raw_parts_mut(data, capacity) }),
+            cursor: std::io::Cursor::new(unsafe { try_slice_mut(data, capacity)? }),
         })
     }