Skip to content

Commit

Permalink
constant_time: Clarify memory safety.
Browse files Browse the repository at this point in the history
The C code correctly handles zero-length inputs by avoiding use
of the pointers, but technically there may be UB since a Rust
dangling pointer isn't a valid C pointer. Avoid the whole
thing by adding a redundant length check on the Rust side.
  • Loading branch information
briansmith committed Jan 26, 2025
1 parent dcc7784 commit e5a1cf1
Showing 1 changed file with 17 additions and 6 deletions.
23 changes: 17 additions & 6 deletions src/constant_time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//! Constant-time operations.
use crate::{c, error};
use core::num::NonZeroUsize;

mod boolmask;
mod leaky;
Expand All @@ -27,18 +28,28 @@ pub(crate) use self::{boolmask::BoolMask, leaky::LeakyWord, word::Word};
/// contents of each, but NOT in constant time with respect to the lengths of
/// `a` and `b`.
pub fn verify_slices_are_equal(a: &[u8], b: &[u8]) -> Result<(), error::Unspecified> {
if a.len() != b.len() {
let len = a.len(); // Arbitrary choice.
if b.len() != len {
return Err(error::Unspecified);
}
let result = unsafe { CRYPTO_memcmp(a.as_ptr(), b.as_ptr(), a.len()) };
match result {
0 => Ok(()),
_ => Err(error::Unspecified),
match NonZeroUsize::new(len) {
Some(len) => {
let a = a.as_ptr();
let b = b.as_ptr();
// SAFETY: `a` and `b` are valid non-null non-dangling pointers to `len`
// bytes.
let result = unsafe { CRYPTO_memcmp(a, b, len) };
match result {
0 => Ok(()),
_ => Err(error::Unspecified),
}
}
None => Ok(()), // Empty slices are equal.
}
}

prefixed_extern! {
fn CRYPTO_memcmp(a: *const u8, b: *const u8, len: c::size_t) -> c::int;
fn CRYPTO_memcmp(a: *const u8, b: *const u8, len: c::NonZero_size_t) -> c::int;
}

pub(crate) fn xor_16(a: [u8; 16], b: [u8; 16]) -> [u8; 16] {
Expand Down

0 comments on commit e5a1cf1

Please sign in to comment.