milksad
/
rust-secp256k1-unsafe-fast
14
0
Fork 0
Code Issues Pull Requests Activity

secp-sys: Use NonNull in API instead of *mut T 

Currently we expect non-null pointers when we take `*mut T` parameters,
however we do not check that the pointers are non-null because we never
set VERIFY in our C build. We can use the `NonNull` type to enforce
no-null-ness as long as we use `NonNull::new`. In a couple of instances
we manually check that a buffer is not empty and therefore that the
pointer to it is non-null so we can safely use `NonNull::new_unchecked`.

Replace mutable pointer parameters `*mut T` (e.g. `*mut c_void`) and
return types with `NonNull<T>`.

Fix #546
This commit is contained in:
Tobin C. Harding 2022-12-01 15:22:24 +11:00
parent 525613902c
commit 9b07e8e8c5
3 changed files with 49 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
secp256k1-sys/src/lib.rs
View File

@ -39,6 +39,7 @@ pub mod types;
pub mod recovery;
use core::{slice, ptr};
use core::ptr::NonNull;
use types::*;
/// Flag for context to enable no precomputation
@ -512,7 +513,7 @@ extern "C" {
// Contexts
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_6_1_context_preallocated_destroy")]
pub fn secp256k1_context_preallocated_destroy(cx: *mut Context);
pub fn secp256k1_context_preallocated_destroy(cx: NonNull<Context>);
// Signatures
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_6_1_ecdsa_signature_parse_der")]
@ -585,16 +586,16 @@ extern "C" {
pub fn secp256k1_context_preallocated_size(flags: c_uint) -> size_t;
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_6_1_context_preallocated_create")]
pub fn secp256k1_context_preallocated_create(prealloc: *mut c_void, flags: c_uint) -> *mut Context;
pub fn secp256k1_context_preallocated_create(prealloc: NonNull<c_void>, flags: c_uint) -> NonNull<Context>;
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_6_1_context_preallocated_clone_size")]
pub fn secp256k1_context_preallocated_clone_size(cx: *const Context) -> size_t;
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_6_1_context_preallocated_clone")]
pub fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context;
pub fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: NonNull<c_void>) -> NonNull<Context>;
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_6_1_context_randomize")]
pub fn secp256k1_context_randomize(cx: *mut Context,
pub fn secp256k1_context_randomize(cx: NonNull<Context>,
seed32: *const c_uchar)
-> c_int;
// Pubkeys
@ -789,7 +790,7 @@ extern "C" {
/// The newly created secp256k1 raw context.
#[cfg(all(feature = "alloc", not(rust_secp_no_symbol_renaming)))]
#[cfg_attr(docsrs, doc(cfg(all(feature = "alloc", not(rust_secp_no_symbol_renaming)))))]
pub unsafe fn secp256k1_context_create(flags: c_uint) -> *mut Context {
pub unsafe fn secp256k1_context_create(flags: c_uint) -> NonNull<Context> {
rustsecp256k1_v0_6_1_context_create(flags)
}
@ -800,7 +801,7 @@ pub unsafe fn secp256k1_context_create(flags: c_uint) -> *mut Context {
#[allow(clippy::missing_safety_doc)] // Documented above.
#[cfg(all(feature = "alloc", not(rust_secp_no_symbol_renaming)))]
#[cfg_attr(docsrs, doc(cfg(all(feature = "alloc", not(rust_secp_no_symbol_renaming)))))]
pub unsafe extern "C" fn rustsecp256k1_v0_6_1_context_create(flags: c_uint) -> *mut Context {
pub unsafe extern "C" fn rustsecp256k1_v0_6_1_context_create(flags: c_uint) -> NonNull<Context> {
use core::mem;
use crate::alloc::alloc;
assert!(ALIGN_TO >= mem::align_of::<usize>());
@ -817,7 +818,8 @@ pub unsafe extern "C" fn rustsecp256k1_v0_6_1_context_create(flags: c_uint) -> *
(ptr as *mut usize).write(bytes);
// We must offset a whole ALIGN_TO in order to preserve the same alignment
// this means we "lose" ALIGN_TO-size_of(usize) for padding.
let ptr = ptr.add(ALIGN_TO) as *mut c_void;
let ptr = ptr.add(ALIGN_TO);
let ptr = NonNull::new_unchecked(ptr as *mut c_void); // Checked above.
secp256k1_context_preallocated_create(ptr, flags)
}
@ -832,7 +834,7 @@ pub unsafe extern "C" fn rustsecp256k1_v0_6_1_context_create(flags: c_uint) -> *
/// `ctx` must be a valid pointer to a block of memory created using [`secp256k1_context_create`].
#[cfg(all(feature = "alloc", not(rust_secp_no_symbol_renaming)))]
#[cfg_attr(docsrs, doc(cfg(all(feature = "alloc", not(rust_secp_no_symbol_renaming)))))]
pub unsafe fn secp256k1_context_destroy(ctx: *mut Context) {
pub unsafe fn secp256k1_context_destroy(ctx: NonNull<Context>) {
rustsecp256k1_v0_6_1_context_destroy(ctx)
}
@ -840,9 +842,10 @@ pub unsafe fn secp256k1_context_destroy(ctx: *mut Context) {
#[allow(clippy::missing_safety_doc)] // Documented above.
#[cfg(all(feature = "alloc", not(rust_secp_no_symbol_renaming)))]
#[cfg_attr(docsrs, doc(cfg(all(feature = "alloc", not(rust_secp_no_symbol_renaming)))))]
pub unsafe extern "C" fn rustsecp256k1_v0_6_1_context_destroy(ctx: *mut Context) {
pub unsafe extern "C" fn rustsecp256k1_v0_6_1_context_destroy(mut ctx: NonNull<Context>) {
use crate::alloc::alloc;
secp256k1_context_preallocated_destroy(ctx);
let ctx: *mut Context = ctx.as_mut();
let ptr = (ctx as *mut u8).sub(ALIGN_TO);
let bytes = (ptr as *mut usize).read();
let layout = alloc::Layout::from_size_align(bytes, ALIGN_TO).unwrap();
@ -966,8 +969,8 @@ mod fuzz_dummy {
extern "C" {
fn rustsecp256k1_v0_6_1_context_preallocated_size(flags: c_uint) -> size_t;
fn rustsecp256k1_v0_6_1_context_preallocated_create(prealloc: *mut c_void, flags: c_uint) -> *mut Context;
fn rustsecp256k1_v0_6_1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context;
fn rustsecp256k1_v0_6_1_context_preallocated_create(prealloc: NonNull<c_void>, flags: c_uint) -> NonNull<Context>;
fn rustsecp256k1_v0_6_1_context_preallocated_clone(cx: *const Context, prealloc: NonNull<c_void>) -> NonNull<Context>;
}
#[cfg(feature = "lowmemory")]
@ -985,7 +988,7 @@ mod fuzz_dummy {
const HAVE_CONTEXT_WORKING: usize = 1;
const HAVE_CONTEXT_DONE: usize = 2;
static mut PREALLOCATED_CONTEXT: [u8; CTX_SIZE] = [0; CTX_SIZE];
pub unsafe fn secp256k1_context_preallocated_create(prealloc: *mut c_void, flags: c_uint) -> *mut Context {
pub unsafe fn secp256k1_context_preallocated_create(prealloc: NonNull<c_void>, flags: c_uint) -> NonNull<Context> {
// While applications should generally avoid creating too many contexts, sometimes fuzzers
// perform tasks repeatedly which real applications may only do rarely. Thus, we want to
// avoid being overly slow here. We do so by having a static context and copying it into
@ -998,9 +1001,9 @@ mod fuzz_dummy {
if have_ctx == HAVE_CONTEXT_NONE {
assert!(rustsecp256k1_v0_6_1_context_preallocated_size(SECP256K1_START_SIGN | SECP256K1_START_VERIFY) + std::mem::size_of::<c_uint>() <= CTX_SIZE);
assert_eq!(rustsecp256k1_v0_6_1_context_preallocated_create(
PREALLOCATED_CONTEXT[..].as_ptr() as *mut c_void,
NonNull::new_unchecked(PREALLOCATED_CONTEXT[..].as_mut_ptr() as *mut c_void),
SECP256K1_START_SIGN | SECP256K1_START_VERIFY),
PREALLOCATED_CONTEXT[..].as_ptr() as *mut Context);
NonNull::new_unchecked(PREALLOCATED_CONTEXT[..].as_mut_ptr() as *mut Context));
assert_eq!(HAVE_PREALLOCATED_CONTEXT.swap(HAVE_CONTEXT_DONE, Ordering::AcqRel),
HAVE_CONTEXT_WORKING);
} else if have_ctx == HAVE_CONTEXT_DONE {
@ -1015,25 +1018,25 @@ mod fuzz_dummy {
std::thread::yield_now();
}
}
ptr::copy_nonoverlapping(PREALLOCATED_CONTEXT[..].as_ptr(), prealloc as *mut u8, CTX_SIZE);
let ptr = (prealloc as *mut u8).add(CTX_SIZE).sub(std::mem::size_of::<c_uint>());
ptr::copy_nonoverlapping(PREALLOCATED_CONTEXT[..].as_ptr(), prealloc.as_ptr() as *mut u8, CTX_SIZE);
let ptr = (prealloc.as_ptr()).add(CTX_SIZE).sub(std::mem::size_of::<c_uint>());
(ptr as *mut c_uint).write(flags);
prealloc as *mut Context
NonNull::new_unchecked(prealloc.as_ptr() as *mut Context)
}
pub unsafe fn secp256k1_context_preallocated_clone_size(_cx: *const Context) -> size_t { CTX_SIZE }
pub unsafe fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context {
pub unsafe fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: NonNull<c_void>) -> NonNull<Context> {
let orig_ptr = (cx as *mut u8).add(CTX_SIZE).sub(std::mem::size_of::<c_uint>());
let new_ptr = (prealloc as *mut u8).add(CTX_SIZE).sub(std::mem::size_of::<c_uint>());
let new_ptr = (prealloc.as_ptr() as *mut u8).add(CTX_SIZE).sub(std::mem::size_of::<c_uint>());
let flags = (orig_ptr as *mut c_uint).read();
(new_ptr as *mut c_uint).write(flags);
rustsecp256k1_v0_6_1_context_preallocated_clone(cx, prealloc)
}
pub unsafe fn secp256k1_context_randomize(cx: *mut Context,
pub unsafe fn secp256k1_context_randomize(cx: NonNull<Context>,
_seed32: *const c_uchar)
-> c_int {
// This function is really slow, and unsuitable for fuzzing
check_context_flags(cx, 0);
check_context_flags(cx.as_ptr(), 0);
1
}

47
src/context.rs
View File

@ -204,18 +204,12 @@ mod alloc_only {
let size = unsafe { ffi::secp256k1_context_preallocated_size(C::FLAGS) };
let layout = alloc::Layout::from_size_align(size, ALIGN_TO).unwrap();
let ptr = unsafe { alloc::alloc(layout) };
if ptr.is_null() {
alloc::handle_alloc_error(layout);
}
let ptr = NonNull::new(ptr as *mut c_void)
.unwrap_or_else(|| alloc::handle_alloc_error(layout));
#[allow(unused_mut)] // ctx is not mutated under some feature combinations.
let mut ctx = Secp256k1 {
ctx: unsafe {
NonNull::new_unchecked(ffi::secp256k1_context_preallocated_create(
ptr as *mut c_void,
C::FLAGS,
))
},
ctx: unsafe { ffi::secp256k1_context_preallocated_create(ptr, C::FLAGS) },
phantom: PhantomData,
};
@ -269,16 +263,11 @@ mod alloc_only {
let size = unsafe { ffi::secp256k1_context_preallocated_clone_size(self.ctx.as_ptr()) };
let layout = alloc::Layout::from_size_align(size, ALIGN_TO).unwrap();
let ptr = unsafe { alloc::alloc(layout) };
if ptr.is_null() {
alloc::handle_alloc_error(layout);
}
let ptr = NonNull::new(ptr as *mut c_void)
.unwrap_or_else(|| alloc::handle_alloc_error(layout));
Secp256k1 {
ctx: unsafe {
NonNull::new_unchecked(ffi::secp256k1_context_preallocated_clone(
self.ctx.as_ptr(),
ptr as *mut c_void,
))
},
ctx: unsafe { ffi::secp256k1_context_preallocated_clone(self.ctx.as_ptr(), ptr) },
phantom: PhantomData,
}
}
@ -327,13 +316,11 @@ impl<'buf, C: Context + 'buf> Secp256k1<C> {
if buf.len() < Self::preallocate_size_gen() {
return Err(Error::NotEnoughMemory);
}
// Safe because buf is not null since it is not empty.
let buf = unsafe { NonNull::new_unchecked(buf.as_mut_c_ptr() as *mut c_void) };
Ok(Secp256k1 {
ctx: unsafe {
NonNull::new_unchecked(ffi::secp256k1_context_preallocated_create(
buf.as_mut_c_ptr() as *mut c_void,
C::FLAGS,
))
},
ctx: unsafe { ffi::secp256k1_context_preallocated_create(buf, AllPreallocated::FLAGS) },
phantom: PhantomData,
})
}
@ -361,9 +348,9 @@ impl<'buf> Secp256k1<AllPreallocated<'buf>> {
/// * Violating these may lead to Undefined Behavior.
///
pub unsafe fn from_raw_all(
raw_ctx: *mut ffi::Context,
raw_ctx: NonNull<ffi::Context>,
) -> ManuallyDrop<Secp256k1<AllPreallocated<'buf>>> {
ManuallyDrop::new(Secp256k1 { ctx: NonNull::new_unchecked(raw_ctx), phantom: PhantomData })
ManuallyDrop::new(Secp256k1 { ctx: raw_ctx, phantom: PhantomData })
}
}
@ -392,9 +379,9 @@ impl<'buf> Secp256k1<SignOnlyPreallocated<'buf>> {
/// * This list *is not* exhaustive, and any violation may lead to Undefined Behavior.
///
pub unsafe fn from_raw_signing_only(
raw_ctx: *mut ffi::Context,
raw_ctx: NonNull<ffi::Context>,
) -> ManuallyDrop<Secp256k1<SignOnlyPreallocated<'buf>>> {
ManuallyDrop::new(Secp256k1 { ctx: NonNull::new_unchecked(raw_ctx), phantom: PhantomData })
ManuallyDrop::new(Secp256k1 { ctx: raw_ctx, phantom: PhantomData })
}
}
@ -423,8 +410,8 @@ impl<'buf> Secp256k1<VerifyOnlyPreallocated<'buf>> {
/// * This list *is not* exhaustive, and any violation may lead to Undefined Behavior.
///
pub unsafe fn from_raw_verification_only(
raw_ctx: *mut ffi::Context,
raw_ctx: NonNull<ffi::Context>,
) -> ManuallyDrop<Secp256k1<VerifyOnlyPreallocated<'buf>>> {
ManuallyDrop::new(Secp256k1 { ctx: NonNull::new_unchecked(raw_ctx), phantom: PhantomData })
ManuallyDrop::new(Secp256k1 { ctx: raw_ctx, phantom: PhantomData })
}
}

17
src/lib.rs
View File

@ -389,7 +389,7 @@ impl<C: Context> Drop for Secp256k1<C> {
fn drop(&mut self) {
unsafe {
let size = ffi::secp256k1_context_preallocated_clone_size(self.ctx.as_ptr());
ffi::secp256k1_context_preallocated_destroy(self.ctx.as_ptr());
ffi::secp256k1_context_preallocated_destroy(self.ctx);
C::deallocate(self.ctx.as_ptr() as _, size);
}
@ -434,7 +434,7 @@ impl<C: Context> Secp256k1<C> {
/// see comment in libsecp256k1 commit d2275795f by Gregory Maxwell.
pub fn seeded_randomize(&mut self, seed: &[u8; 32]) {
unsafe {
let err = ffi::secp256k1_context_randomize(self.ctx.as_ptr(), seed.as_c_ptr());
let err = ffi::secp256k1_context_randomize(self.ctx, seed.as_c_ptr());
// This function cannot fail; it has an error return for future-proofing.
// We do not expose this error since it is impossible to hit, and we have
// precedent for not exposing impossible errors (for example in
@ -558,12 +558,11 @@ mod tests {
let ctx_sign = unsafe { ffi::secp256k1_context_create(SignOnlyPreallocated::FLAGS) };
let ctx_vrfy = unsafe { ffi::secp256k1_context_create(VerifyOnlyPreallocated::FLAGS) };
let full: Secp256k1<AllPreallocated> =
Secp256k1 { ctx: unsafe { NonNull::new_unchecked(ctx_full) }, phantom: PhantomData };
let full: Secp256k1<AllPreallocated> = Secp256k1 { ctx: ctx_full, phantom: PhantomData };
let sign: Secp256k1<SignOnlyPreallocated> =
Secp256k1 { ctx: unsafe { NonNull::new_unchecked(ctx_sign) }, phantom: PhantomData };
Secp256k1 { ctx: ctx_sign, phantom: PhantomData };
let vrfy: Secp256k1<VerifyOnlyPreallocated> =
Secp256k1 { ctx: unsafe { NonNull::new_unchecked(ctx_vrfy) }, phantom: PhantomData };
Secp256k1 { ctx: ctx_vrfy, phantom: PhantomData };
let (sk, pk) = full.generate_keypair(&mut rand::thread_rng());
let msg = Message::from_slice(&[2u8; 32]).unwrap();
@ -593,9 +592,9 @@ mod tests {
let ctx_sign = Secp256k1::signing_only();
let ctx_vrfy = Secp256k1::verification_only();
let mut full = unsafe { Secp256k1::from_raw_all(ctx_full.ctx.as_ptr()) };
let mut sign = unsafe { Secp256k1::from_raw_signing_only(ctx_sign.ctx.as_ptr()) };
let mut vrfy = unsafe { Secp256k1::from_raw_verification_only(ctx_vrfy.ctx.as_ptr()) };
let mut full = unsafe { Secp256k1::from_raw_all(ctx_full.ctx) };
let mut sign = unsafe { Secp256k1::from_raw_signing_only(ctx_sign.ctx) };
let mut vrfy = unsafe { Secp256k1::from_raw_verification_only(ctx_vrfy.ctx) };
let (sk, pk) = full.generate_keypair(&mut rand::thread_rng());
let msg = Message::from_slice(&[2u8; 32]).unwrap();