From 2139aff0d705142ffb40883023bf47d4978dae70 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 27 Jun 2025 21:54:52 +0000 Subject: [PATCH 1/4] secp256k1-sys: fix type signature in secp256k1_ec_pubkey_sort In #794 I accidentally changed a *mut to *const. This is incorrect and unsound. --- secp256k1-sys/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index db1ad80ad..6a58f54d3 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -915,7 +915,7 @@ extern "C" { #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_11_ec_pubkey_sort")] pub fn secp256k1_ec_pubkey_sort( ctx: *const Context, - pubkeys: *const *const PublicKey, + pubkeys: *mut *const PublicKey, n_pubkeys: size_t, ) -> c_int; } From 44bcb893053d96bf5f082c6acd45e89226128300 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 27 Jun 2025 21:50:03 +0000 Subject: [PATCH 2/4] ellswift: remove unused `data` argument from `shared_secret` We take an optional `data` argument to `shared_secret`, but the upstream function we call never uses it. However, this argument *does* use the CPtr impl on &[T] in order to obtain a pointer to pass across the FFI boundary. This impl is very dangerous, and its use here is sound only because the resulting pointer is never used. See https://github.com/rust-bitcoin/rust-secp256k1/pull/627#discussion_r2172910653 --- src/ellswift.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/ellswift.rs b/src/ellswift.rs index 11877fb03..73a6688f3 100644 --- a/src/ellswift.rs +++ b/src/ellswift.rs @@ -167,8 +167,8 @@ impl ElligatorSwift { /// let alice_es = ElligatorSwift::from_seckey(&secp, alice_sk, None); /// let bob_es = ElligatorSwift::from_seckey(&secp, bob_sk, None); /// - /// let alice_shared_secret = ElligatorSwift::shared_secret(alice_es, bob_es, alice_sk, Party::Initiator, None); - /// let bob_shared_secret = ElligatorSwift::shared_secret(alice_es, bob_es, bob_sk, Party::Responder, None); + /// let alice_shared_secret = ElligatorSwift::shared_secret(alice_es, bob_es, alice_sk, Party::Initiator); + /// let bob_shared_secret = ElligatorSwift::shared_secret(alice_es, bob_es, bob_sk, Party::Responder); /// /// assert_eq!(alice_shared_secret, bob_shared_secret); /// # } @@ -178,7 +178,6 @@ impl ElligatorSwift { ellswift_b: ElligatorSwift, secret_key: SecretKey, party: impl Into, - data: Option<&[u8]>, ) -> ElligatorSwiftSharedSecret { let mut shared_secret = [0u8; 32]; let p: Party = party.into(); @@ -191,7 +190,7 @@ impl ElligatorSwift { secret_key.as_c_ptr(), p.to_ffi_int(), ffi::secp256k1_ellswift_xdh_hash_function_bip324, - data.as_c_ptr() as *mut c_void, + ptr::null_mut(), ); debug_assert_eq!(ret, 1); } @@ -631,7 +630,7 @@ mod tests { let sec_key = SecretKey::from_byte_array(my_secret).unwrap(); let initiator = if initiator == 0 { Party::Responder } else { Party::Initiator }; - let shared = ElligatorSwift::shared_secret(el_a, el_b, sec_key, initiator, None); + let shared = ElligatorSwift::shared_secret(el_a, el_b, sec_key, initiator); assert_eq!(shared.0, shared_secret); } From 2cef56192f8107ab4ee2c8f5e1131708769b82fc Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 27 Jun 2025 21:59:03 +0000 Subject: [PATCH 3/4] key.rs: clean up some pointer mangling in sort_pubkeys() In sort_pubkeys() we convert a slice of PublicKeys to a slice of ffi::PublicKeys. This is OK, because we have repr(transparent) allowing us to do this. But we do so in an unnecessarily-complicated way, which winds up going through the CPtr implementation on &[u8], and which I think may technically be unsound. Simplify it and avoid using this impl. The next commit will remove it entirely. --- src/key.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/key.rs b/src/key.rs index 3aef6302b..e446afe7f 100644 --- a/src/key.rs +++ b/src/key.rs @@ -1644,11 +1644,9 @@ impl Secp256k1 { pub fn sort_pubkeys(&self, pubkeys: &mut [&PublicKey]) { let cx = self.ctx().as_ptr(); unsafe { - let mut pubkeys_ref = core::slice::from_raw_parts( - pubkeys.as_c_ptr() as *mut *const ffi::PublicKey, - pubkeys.len(), - ); - if secp256k1_ec_pubkey_sort(cx, pubkeys_ref.as_mut_c_ptr(), pubkeys_ref.len()) == 0 { + // SAFETY: `PublicKey` has repr(transparent) so we can convert to `ffi::PublicKey` + let pubkeys_ptr = pubkeys.as_mut_c_ptr() as *mut *const ffi::PublicKey; + if secp256k1_ec_pubkey_sort(cx, pubkeys_ptr, pubkeys.len()) == 0 { unreachable!("Invalid public keys for sorting function") } } From debae9017592c4659bb0d64294d1cdadbf8e652f Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 27 Jun 2025 21:59:03 +0000 Subject: [PATCH 4/4] secp256k1-sys: remove CPtr impl for &[T] --- secp256k1-sys/src/lib.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index 6a58f54d3..aaae6c949 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -1388,25 +1388,6 @@ impl CPtr for [T] { } } -impl CPtr for &[T] { - type Target = T; - fn as_c_ptr(&self) -> *const Self::Target { - if self.is_empty() { - ptr::null() - } else { - self.as_ptr() - } - } - - fn as_mut_c_ptr(&mut self) -> *mut Self::Target { - if self.is_empty() { - ptr::null_mut() - } else { - self.as_ptr() as *mut Self::Target - } - } -} - impl CPtr for [u8; 32] { type Target = u8; fn as_c_ptr(&self) -> *const Self::Target { self.as_ptr() }