From dd194b6be880b60a278925d4d258b68af0fdb5c1 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 2 Dec 2022 12:58:32 +0000 Subject: [PATCH 1/3] context: introduce unsafe `PreallocatedContext` trait Fixes unsoundness in `preallocated_gen_new` which previously did not properly constrain the lifetime of the buffer used to back the context object. We introduce an unsafe marker trait, and impl it for our existing preallocated-context markers. Annoyingly the trait has to be public even though it should never be used directly, and is only used alongside the sealed `Context` trait, so it is de-facto sealed itself. Fixes #543 --- src/context.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/context.rs b/src/context.rs index 2248ccdff..8d5dbadb0 100644 --- a/src/context.rs +++ b/src/context.rs @@ -300,8 +300,16 @@ unsafe impl<'buf> Context for AllPreallocated<'buf> { } } -impl<'buf, C: Context + 'buf> Secp256k1 { - /// Lets you create a context with preallocated buffer in a generic manner(sign/verify/all) +/// Trait marking that a particular context object internally points to +/// memory that must outlive `'a` +pub unsafe trait PreallocatedContext<'a> {} + +unsafe impl<'buf> PreallocatedContext<'buf> for AllPreallocated<'buf> {} +unsafe impl<'buf> PreallocatedContext<'buf> for SignOnlyPreallocated<'buf> {} +unsafe impl<'buf> PreallocatedContext<'buf> for VerifyOnlyPreallocated<'buf> {} + +impl<'buf, C: Context + PreallocatedContext<'buf>> Secp256k1 { + /// Lets you create a context with a preallocated buffer in a generic manner (sign/verify/all). pub fn preallocated_gen_new(buf: &'buf mut [AlignedType]) -> Result, Error> { #[cfg(target_arch = "wasm32")] ffi::types::sanity_checks_for_wasm(); From 0a696b2d55dfaee60fd2b7f5c1d47577c10b4632 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 5 Dec 2022 13:59:01 +1100 Subject: [PATCH 2/3] Add saftey docs for PreallocatedContext trait Add `# Safety` section to the rustdocs of the `PrealocatedContext` trait. This change was back ported manually (instead of directly cherry-picking the patch) so as to add a blank newline after the heading as is customary. Original: `commit 1e6eb6cb4dd435d062e9d4d16a2218c9033d5c78` --- src/context.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/context.rs b/src/context.rs index 8d5dbadb0..e52884e1b 100644 --- a/src/context.rs +++ b/src/context.rs @@ -302,6 +302,12 @@ unsafe impl<'buf> Context for AllPreallocated<'buf> { /// Trait marking that a particular context object internally points to /// memory that must outlive `'a` +/// +/// # Safety +/// +/// This trait is used internally to gate which context markers can safely +/// be used with the `preallocated_gen_new` function. Do not implement it +/// on your own structures. pub unsafe trait PreallocatedContext<'a> {} unsafe impl<'buf> PreallocatedContext<'buf> for AllPreallocated<'buf> {} From 5c6225e7b5062d83f18c67e3eaf13341e5bf6599 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 6 Dec 2022 08:20:12 +1100 Subject: [PATCH 3/3] Bump version to 0.24.2 Done after backport to fix unsoundness issue. --- CHANGELOG.md | 5 +++++ Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49a369c06..914efff7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ + +# 0.24.2 - 2022-12-05 + +* Backport [fix soundness issue with `preallocated_gen_new`](https://github.com/rust-bitcoin/rust-secp256k1/pull/548) + # 0.24.1 - 2022-10-25 * [Fix broken deserialization logic of `KeyPair`](https://github.com/rust-bitcoin/rust-secp256k1/issues/491) that previously always panicked. After the patch deserialization only panics if neither the `global-context` nor the `alloc` (default) feature is active. diff --git a/Cargo.toml b/Cargo.toml index 7cef32d43..59a8d92fc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "secp256k1" -version = "0.24.1" +version = "0.24.2" authors = [ "Dawid Ciężarkiewicz ", "Andrew Poelstra " ] license = "CC0-1.0"