From fe688ada65b6b4a40ed0740b6f4cbe688c044473 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Wed, 27 Nov 2019 17:36:06 +0200 Subject: [PATCH 1/2] Make the Context trait unimplementable --- src/context.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/context.rs b/src/context.rs index e8eba5d..583b5f9 100644 --- a/src/context.rs +++ b/src/context.rs @@ -8,8 +8,8 @@ use Secp256k1; pub use self::std_only::*; /// A trait for all kinds of Context's that Lets you define the exact flags and a function to deallocate memory. -/// * DO NOT * implement it for your own types. -pub unsafe trait Context { +/// It shouldn't be possible to implement this for types outside this crate. +pub unsafe trait Context : private::Sealed { /// Flags for the ffi. const FLAGS: c_uint; /// A constant description of the context. @@ -39,8 +39,24 @@ pub struct AllPreallocated<'buf> { phantom: PhantomData<&'buf ()>, } +mod private { + use super::*; + // A trick to prevent users from implementing a trait. + // on one hand this trait is public, on the other it's in a private module + // so it's not visible to anyone besides it's parent (the context module) + pub trait Sealed {} + + impl<'buf> Sealed for AllPreallocated<'buf> {} + impl<'buf> Sealed for VerifyOnlyPreallocated<'buf> {} + impl<'buf> Sealed for SignOnlyPreallocated<'buf> {} +} + #[cfg(feature = "std")] mod std_only { + impl private::Sealed for SignOnly {} + impl private::Sealed for All {} + impl private::Sealed for VerifyOnly {} + use super::*; /// Represents the set of capabilities needed for signing. From 9522f7e4a4c2036923b71df03411456a3b258a9e Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Wed, 27 Nov 2019 17:42:01 +0200 Subject: [PATCH 2/2] Make Context::deallocate unsafe fn --- src/context.rs | 21 ++++++++++----------- src/lib.rs | 6 ++++-- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/context.rs b/src/context.rs index 583b5f9..4907e0f 100644 --- a/src/context.rs +++ b/src/context.rs @@ -15,7 +15,7 @@ pub unsafe trait Context : private::Sealed { /// A constant description of the context. const DESCRIPTION: &'static str; /// A function to deallocate the memory when the context is dropped. - fn deallocate(ptr: *mut [u8]); + unsafe fn deallocate(ptr: *mut [u8]); } /// Marker trait for indicating that an instance of `Secp256k1` can be used for signing. @@ -78,8 +78,8 @@ mod std_only { const FLAGS: c_uint = ffi::SECP256K1_START_SIGN; const DESCRIPTION: &'static str = "signing only"; - fn deallocate(ptr: *mut [u8]) { - let _ = unsafe { Box::from_raw(ptr) }; + unsafe fn deallocate(ptr: *mut [u8]) { + let _ = Box::from_raw(ptr); } } @@ -87,8 +87,8 @@ mod std_only { const FLAGS: c_uint = ffi::SECP256K1_START_VERIFY; const DESCRIPTION: &'static str = "verification only"; - fn deallocate(ptr: *mut [u8]) { - let _ = unsafe { Box::from_raw(ptr) }; + unsafe fn deallocate(ptr: *mut [u8]) { + let _ = Box::from_raw(ptr); } } @@ -96,8 +96,8 @@ mod std_only { const FLAGS: c_uint = VerifyOnly::FLAGS | SignOnly::FLAGS; const DESCRIPTION: &'static str = "all capabilities"; - fn deallocate(ptr: *mut [u8]) { - let _ = unsafe { Box::from_raw(ptr) }; + unsafe fn deallocate(ptr: *mut [u8]) { + let _ = Box::from_raw(ptr); } } @@ -152,7 +152,6 @@ mod std_only { } } } - } impl<'buf> Signing for SignOnlyPreallocated<'buf> {} @@ -165,7 +164,7 @@ unsafe impl<'buf> Context for SignOnlyPreallocated<'buf> { const FLAGS: c_uint = ffi::SECP256K1_START_SIGN; const DESCRIPTION: &'static str = "signing only"; - fn deallocate(_ptr: *mut [u8]) { + unsafe fn deallocate(_ptr: *mut [u8]) { // Allocated by the user } } @@ -174,7 +173,7 @@ unsafe impl<'buf> Context for VerifyOnlyPreallocated<'buf> { const FLAGS: c_uint = ffi::SECP256K1_START_VERIFY; const DESCRIPTION: &'static str = "verification only"; - fn deallocate(_ptr: *mut [u8]) { + unsafe fn deallocate(_ptr: *mut [u8]) { // Allocated by the user } } @@ -183,7 +182,7 @@ unsafe impl<'buf> Context for AllPreallocated<'buf> { const FLAGS: c_uint = SignOnlyPreallocated::FLAGS | VerifyOnlyPreallocated::FLAGS; const DESCRIPTION: &'static str = "all capabilities"; - fn deallocate(_ptr: *mut [u8]) { + unsafe fn deallocate(_ptr: *mut [u8]) { // Allocated by the user } } diff --git a/src/lib.rs b/src/lib.rs index 854b166..2926bfa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -571,8 +571,10 @@ impl Eq for Secp256k1 { } impl Drop for Secp256k1 { fn drop(&mut self) { - unsafe { ffi::secp256k1_context_preallocated_destroy(self.ctx) }; - C::deallocate(self.buf); + unsafe { + ffi::secp256k1_context_preallocated_destroy(self.ctx); + C::deallocate(self.buf); + } } }