Merge rust-bitcoin/rust-secp256k1#561: Fully describe safety requirements

e705bcffb5 Fully describe safety requirements (Tobin C. Harding)

Pull request description:

  Currently we have a wildcard on safety requirements, saying more or less "plus a bunch of other stuff we don't mention". This is not helpful.

  Attempt to fully describe the safety requirements of creating a context from a raw context (all, signing only, and verification only).

  Fix: #544

  ## Note

  This is best effort only, will require some thought to review. To do this I read https://doc.rust-lang.org/reference/behavior-considered-undefined.html and then I flicked through `depend/secp256k1/src/secp256k1.c` and `util.h` to look for things that could cause things in the linked to list of UB.

ACKs for top commit:
  apoelstra:
    ACK e705bcffb5
  Kixunil:
    ACK e705bcffb5

Tree-SHA512: 0180d196f6d528e3c7a06da54ef58d015df19c351d98030453ae5c5e62e0565797b06169f27f5d8b40ea0b9adba377cadd45dd306c8213d0bdc98b20651766c7
This commit is contained in:
Andrew Poelstra 2023-02-06 13:50:41 +00:00
commit 6ec968a522
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
1 changed files with 19 additions and 26 deletions

View File

@ -352,15 +352,22 @@ impl<'buf> Secp256k1<AllPreallocated<'buf>> {
/// Creates a context from a raw context. /// Creates a context from a raw context.
/// ///
/// # Safety /// The returned [`core::mem::ManuallyDrop`] context will never deallocate the memory pointed to
/// This is highly unsafe, due to the number of conditions that aren't checked. /// by `raw_ctx` nor destroy the context. This may lead to memory leaks. `ManuallyDrop::drop`
/// * `raw_ctx` needs to be a valid Secp256k1 context pointer. /// (or [`core::ptr::drop_in_place`]) will only destroy the context; the caller is required to
/// that was generated by *exactly* the same code/version of the libsecp256k1 used here. /// free the memory.
/// * The capabilities (All/SignOnly/VerifyOnly) of the context *must* match the flags passed to libsecp256k1
/// when generating the context.
/// * The user must handle the freeing of the context(using the correct functions) by himself.
/// * Violating these may lead to Undefined Behavior.
/// ///
/// # Safety
///
/// This is highly unsafe due to a number of conditions that aren't checked, specifically:
///
/// * `raw_ctx` must be a valid pointer (live, aligned...) to memory that was initialized by
/// `secp256k1_context_preallocated_create` (either called directly or from this library by
/// one of the context creation methods - all of which call it internally).
/// * The version of `libsecp256k1` used to create `raw_ctx` must be **exactly the one linked
/// into this library**.
/// * The lifetime of the `raw_ctx` pointer must outlive `'buf`.
/// * `raw_ctx` must point to writable memory (cannot be `ffi::secp256k1_context_no_precomp`).
pub unsafe fn from_raw_all( pub unsafe fn from_raw_all(
raw_ctx: NonNull<ffi::Context>, raw_ctx: NonNull<ffi::Context>,
) -> ManuallyDrop<Secp256k1<AllPreallocated<'buf>>> { ) -> ManuallyDrop<Secp256k1<AllPreallocated<'buf>>> {
@ -380,18 +387,11 @@ impl<'buf> Secp256k1<SignOnlyPreallocated<'buf>> {
#[inline] #[inline]
pub fn preallocate_signing_size() -> usize { Self::preallocate_size_gen() } pub fn preallocate_signing_size() -> usize { Self::preallocate_size_gen() }
/// Creates a context from a raw context. /// Creates a context from a raw context that can only be used for signing.
/// ///
/// # Safety /// # Safety
/// ///
/// This is highly unsafe, due to the number of conditions that aren't checked. /// Please see [`Secp256k1::from_raw_all`] for full documentation and safety requirements.
/// * `raw_ctx` needs to be a valid Secp256k1 context pointer.
/// that was generated by *exactly* the same code/version of the libsecp256k1 used here.
/// * The capabilities (All/SignOnly/VerifyOnly) of the context *must* match the flags passed to libsecp256k1
/// when generating the context.
/// * The user must handle the freeing of the context(using the correct functions) by himself.
/// * This list *is not* exhaustive, and any violation may lead to Undefined Behavior.
///
pub unsafe fn from_raw_signing_only( pub unsafe fn from_raw_signing_only(
raw_ctx: NonNull<ffi::Context>, raw_ctx: NonNull<ffi::Context>,
) -> ManuallyDrop<Secp256k1<SignOnlyPreallocated<'buf>>> { ) -> ManuallyDrop<Secp256k1<SignOnlyPreallocated<'buf>>> {
@ -411,18 +411,11 @@ impl<'buf> Secp256k1<VerifyOnlyPreallocated<'buf>> {
#[inline] #[inline]
pub fn preallocate_verification_size() -> usize { Self::preallocate_size_gen() } pub fn preallocate_verification_size() -> usize { Self::preallocate_size_gen() }
/// Creates a context from a raw context. /// Creates a context from a raw context that can only be used for verification.
/// ///
/// # Safety /// # Safety
/// ///
/// This is highly unsafe, due to the number of conditions that aren't checked. /// Please see [`Secp256k1::from_raw_all`] for full documentation and safety requirements.
/// * `raw_ctx` needs to be a valid Secp256k1 context pointer.
/// that was generated by *exactly* the same code/version of the libsecp256k1 used here.
/// * The capabilities (All/SignOnly/VerifyOnly) of the context *must* match the flags passed to libsecp256k1
/// when generating the context.
/// * The user must handle the freeing of the context(using the correct functions) by himself.
/// * This list *is not* exhaustive, and any violation may lead to Undefined Behavior.
///
pub unsafe fn from_raw_verification_only( pub unsafe fn from_raw_verification_only(
raw_ctx: NonNull<ffi::Context>, raw_ctx: NonNull<ffi::Context>,
) -> ManuallyDrop<Secp256k1<VerifyOnlyPreallocated<'buf>>> { ) -> ManuallyDrop<Secp256k1<VerifyOnlyPreallocated<'buf>>> {