Skip to content

Commit 45ed1c7

Browse files
kornelskikornelski
committed
Safer slices
1 parent 06ca1fd commit 45ed1c7

8 files changed

Lines changed: 107 additions & 97 deletions

File tree

‎boring/src/asn1.rs‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,23 @@
2424
//! use boring::asn1::Asn1Time;
2525
//! let tomorrow = Asn1Time::days_from_now(1);
2626
//! ```
27-
use crate::ffi;
2827
use foreign_types::{ForeignType, ForeignTypeRef};
2928
use libc::{c_int, c_long, time_t};
3029
use std::cmp::Ordering;
3130
use std::ffi::CString;
3231
use std::fmt;
3332
use std::ptr;
34-
use std::slice;
3533
use std::str;
3634

3735
use crate::bio::MemBio;
3836
use crate::bn::{BigNum, BigNumRef};
3937
use crate::error::ErrorStack;
38+
use crate::ffi;
4039
use crate::nid::Nid;
4140
use crate::stack::Stackable;
4241
use crate::string::OpensslString;
43-
use crate::{cvt, cvt_p};
42+
use crate::try_slice;
43+
use crate::{cvt, cvt_n, cvt_p};
4444
use openssl_macros::corresponds;
4545

4646
foreign_type_and_impl_send_sync! {
@@ -403,11 +403,7 @@ impl Asn1StringRef {
403403
pub fn as_utf8(&self) -> Result<OpensslString, ErrorStack> {
404404
unsafe {
405405
let mut ptr = ptr::null_mut();
406-
let len = ffi::ASN1_STRING_to_UTF8(&mut ptr, self.as_ptr());
407-
if len < 0 {
408-
return Err(ErrorStack::get());
409-
}
410-
406+
cvt_n(ffi::ASN1_STRING_to_UTF8(&mut ptr, self.as_ptr()))?;
411407
Ok(OpensslString::from_ptr(ptr.cast()))
412408
}
413409
}
@@ -421,7 +417,7 @@ impl Asn1StringRef {
421417
#[corresponds(ASN1_STRING_get0_data)]
422418
#[must_use]
423419
pub fn as_slice(&self) -> &[u8] {
424-
unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr()), self.len()) }
420+
unsafe { try_slice(ASN1_STRING_get0_data(self.as_ptr()), self.len()).unwrap_or(&[]) }
425421
}
426422

427423
/// Returns the number of bytes in the string.
@@ -529,13 +525,7 @@ impl Asn1BitStringRef {
529525
#[corresponds(ASN1_STRING_get0_data)]
530526
#[must_use]
531527
pub fn as_slice(&self) -> &[u8] {
532-
unsafe {
533-
let ptr = ASN1_STRING_get0_data(self.as_ptr().cast());
534-
if ptr.is_null() {
535-
return &[];
536-
}
537-
slice::from_raw_parts(ptr, self.len())
538-
}
528+
unsafe { try_slice(ASN1_STRING_get0_data(self.as_ptr().cast()), self.len()).unwrap_or(&[]) }
539529
}
540530

541531
/// Returns the Asn1BitString as a str, if possible.

‎boring/src/bio.rs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
use std::marker::PhantomData;
22
use std::ptr;
3-
use std::slice;
43

5-
use crate::cvt_p;
64
use crate::error::ErrorStack;
75
use crate::ffi;
86
use crate::ffi::BIO_new_mem_buf;
9-
use crate::try_int;
7+
use crate::{cvt, cvt_p};
8+
use crate::{try_int, try_slice};
109

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

@@ -54,14 +53,17 @@ impl MemBio {
5453
self.0
5554
}
5655

56+
/// An empty slice may indicate an error, use [`Self::try_get_buf`] instead.
5757
pub fn get_buf(&self) -> &[u8] {
58+
self.try_get_buf().unwrap_or(&[])
59+
}
60+
61+
pub fn try_get_buf(&self) -> Result<&[u8], ErrorStack> {
5862
unsafe {
59-
let mut ptr = ptr::null_mut();
60-
let len = ffi::BIO_get_mem_data(self.0, &mut ptr);
61-
if ptr.is_null() || len < 0 {
62-
return &[];
63-
}
64-
slice::from_raw_parts(ptr.cast_const().cast(), len as usize)
63+
let mut ptr: *const u8 = ptr::null_mut();
64+
let mut len = 0;
65+
cvt(ffi::BIO_mem_contents(self.0, &mut ptr, &mut len))?;
66+
try_slice(ptr, len).ok_or_else(|| ErrorStack::internal_error_str("invalid slice"))
6567
}
6668
}
6769
}

‎boring/src/lib.rs‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,11 +170,11 @@ fn cvt_0(r: usize) -> Result<(), ErrorStack> {
170170
}
171171
}
172172

173-
fn cvt_0i(r: c_int) -> Result<c_int, ErrorStack> {
173+
fn cvt_0i(r: c_int) -> Result<(), ErrorStack> {
174174
if r == 0 {
175175
Err(ErrorStack::get())
176176
} else {
177-
Ok(r)
177+
Ok(())
178178
}
179179
}
180180

@@ -201,6 +201,48 @@ fn cvt_n(r: c_int) -> Result<c_int, ErrorStack> {
201201
}
202202
}
203203

204+
unsafe fn try_slice<'a, T: Sized + 'static, L: TryInto<usize> + Copy + 'static>(
205+
ptr: *const T,
206+
len_in_items: L,
207+
) -> Option<&'a [T]> {
208+
safe_slice_size::<T, L>(len_in_items)
209+
.filter(|_| !ptr.is_null())
210+
.map(|len| {
211+
// C/C++ may assume the pointer can be anything if it's not dereferenced, which isn't true in Rust
212+
if len > 0 {
213+
std::slice::from_raw_parts(ptr, len)
214+
} else {
215+
&[]
216+
}
217+
})
218+
}
219+
220+
unsafe fn try_slice_mut<'a, T: Sized + 'static, L: TryInto<usize> + Copy + 'static>(
221+
ptr: *mut T,
222+
len_in_items: L,
223+
) -> Result<&'a mut [T], ErrorStack> {
224+
safe_slice_size::<T, L>(len_in_items)
225+
.filter(|_| !ptr.is_null())
226+
.map(|len| {
227+
if len > 0 {
228+
std::slice::from_raw_parts_mut(ptr, len)
229+
} else {
230+
&mut []
231+
}
232+
})
233+
.ok_or_else(|| ErrorStack::internal_error_str("invalid slice"))
234+
}
235+
236+
fn safe_slice_size<T: Sized, L: TryInto<usize>>(len_in_items: L) -> Option<usize> {
237+
let len = len_in_items.try_into().ok()?;
238+
// it's UB to have larger allocation size in Rust
239+
if len.checked_mul(size_of::<T>())? < isize::MAX as usize {
240+
Some(len)
241+
} else {
242+
None
243+
}
244+
}
245+
204246
fn try_int<F, T>(from: F) -> Result<T, ErrorStack>
205247
where
206248
F: TryInto<T> + Send + Sync + Copy + 'static,

‎boring/src/pkey.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ where
235235
pub fn raw_public_key_len(&self) -> Result<usize, ErrorStack> {
236236
unsafe {
237237
let mut size = 0;
238-
_ = cvt_0i(ffi::EVP_PKEY_get_raw_public_key(
238+
cvt_0i(ffi::EVP_PKEY_get_raw_public_key(
239239
self.as_ptr(),
240240
std::ptr::null_mut(),
241241
&mut size,
@@ -251,7 +251,7 @@ where
251251
pub fn raw_public_key<'a>(&self, out: &'a mut [u8]) -> Result<&'a [u8], ErrorStack> {
252252
unsafe {
253253
let mut size = out.len();
254-
_ = cvt_0i(ffi::EVP_PKEY_get_raw_public_key(
254+
cvt_0i(ffi::EVP_PKEY_get_raw_public_key(
255255
self.as_ptr(),
256256
out.as_mut_ptr(),
257257
&mut size,
@@ -303,7 +303,7 @@ where
303303
pub fn raw_private_key_len(&self) -> Result<usize, ErrorStack> {
304304
unsafe {
305305
let mut size = 0;
306-
_ = cvt_0i(ffi::EVP_PKEY_get_raw_private_key(
306+
cvt_0i(ffi::EVP_PKEY_get_raw_private_key(
307307
self.as_ptr(),
308308
std::ptr::null_mut(),
309309
&mut size,
@@ -319,7 +319,7 @@ where
319319
pub fn raw_private_key<'a>(&self, out: &'a mut [u8]) -> Result<&'a [u8], ErrorStack> {
320320
unsafe {
321321
let mut size = out.len();
322-
_ = cvt_0i(ffi::EVP_PKEY_get_raw_private_key(
322+
cvt_0i(ffi::EVP_PKEY_get_raw_private_key(
323323
self.as_ptr(),
324324
out.as_mut_ptr(),
325325
&mut size,

‎boring/src/ssl/bio.rs‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ use std::io;
88
use std::io::prelude::*;
99
use std::panic::{catch_unwind, AssertUnwindSafe};
1010
use std::ptr;
11-
use std::slice;
1211

13-
use crate::cvt_p;
1412
use crate::error::ErrorStack;
13+
use crate::{cvt_p, try_int};
14+
use crate::{try_slice, try_slice_mut};
1515

1616
pub struct StreamState<S> {
1717
pub stream: S,
@@ -106,7 +106,9 @@ unsafe extern "C" fn bwrite<S: Write>(bio: *mut BIO, buf: *const c_char, len: c_
106106
};
107107

108108
let state = state::<S>(bio);
109-
let buf = slice::from_raw_parts(buf.cast(), len);
109+
let Some(buf) = try_slice(buf.cast(), len) else {
110+
return -1;
111+
};
110112

111113
match catch_unwind(AssertUnwindSafe(|| state.stream.write(buf))) {
112114
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_
127129
unsafe extern "C" fn bread<S: Read>(bio: *mut BIO, buf: *mut c_char, len: c_int) -> c_int {
128130
BIO_clear_retry_flags(bio);
129131

130-
let Ok(len) = usize::try_from(len) else {
131-
return -1;
132-
};
133-
134132
let state = state::<S>(bio);
135-
let buf = slice::from_raw_parts_mut(buf.cast(), len);
136133

137-
match catch_unwind(AssertUnwindSafe(|| state.stream.read(buf))) {
138-
Ok(Ok(len)) => len as c_int,
134+
let buf = try_int(len)
135+
.and_then(|len: usize| unsafe { try_slice_mut(buf.cast(), len) })
136+
.map_err(io::Error::other);
137+
138+
let res = catch_unwind(AssertUnwindSafe(|| {
139+
state
140+
.stream
141+
.read(buf?)
142+
.and_then(|len| c_int::try_from(len).map_err(io::Error::other))
143+
}));
144+
match res {
145+
Ok(Ok(len)) => len,
139146
Ok(Err(err)) => {
140147
if retriable_error(&err) {
141148
BIO_set_retry_read(bio);

‎boring/src/ssl/callbacks.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::hmac::HmacCtxRef;
1212
use crate::ssl::TicketKeyCallbackResult;
1313
use crate::symm::CipherCtxRef;
1414
use crate::x509::{X509StoreContext, X509StoreContextRef};
15+
use crate::{try_slice, try_slice_mut};
1516
use foreign_types::ForeignType;
1617
use foreign_types::ForeignTypeRef;
1718
use libc::{c_char, c_int, c_uchar, c_uint, c_void};
@@ -666,7 +667,9 @@ where
666667
.ex_data(SslContext::cached_ex_index::<C>())
667668
.expect("BUG: certificate compression missed");
668669

669-
let input_slice = unsafe { std::slice::from_raw_parts(input, input_len) };
670+
let Some(input_slice) = (unsafe { try_slice(input, input_len) }) else {
671+
return 0;
672+
};
670673
let mut writer = CryptoByteBuilder::from_ptr(out);
671674
if compressor.compress(input_slice, &mut writer).is_err() {
672675
return 0;
@@ -755,7 +758,7 @@ impl<'a> CryptoBufferBuilder<'a> {
755758
let buffer = unsafe { crate::cvt_p(ffi::CRYPTO_BUFFER_alloc(&mut data, capacity))? };
756759
Ok(CryptoBufferBuilder {
757760
buffer,
758-
cursor: std::io::Cursor::new(unsafe { std::slice::from_raw_parts_mut(data, capacity) }),
761+
cursor: std::io::Cursor::new(unsafe { try_slice_mut(data, capacity)? }),
759762
})
760763
}
761764

0 commit comments

Comments
 (0)