From c387305b80d7d94a7375e923dea1be29191e4efd Mon Sep 17 00:00:00 2001 From: mitchmindtree Date: Sat, 11 Feb 2023 02:19:54 +1100 Subject: [PATCH 1/3] Refactor of `fuel-crypto` for clarity around safety This removes many of the unsafe constructors exposed in the `fuel- crypto` API, some due to the implementations not actually being unsafe, and some due to a revised implementation that safely encapsulates the unsafe behaviour. The `Message` and `Signature` `from_bytes` and `from_bytes_ref` constructors have been documented to clarify expectations that they expect signature or cryptographically hashed data respectively. The `PublicKey` and `SecretKey` `from_bytes_unchecked` constructors have been made private in order to ensure holding an instance of one of these types guarantees their inner byte arrays at least satisfies the curve. The original, custom curve-checking functions have been removed in favour of using the secp256k1 library constructors directly which share the same implementation but with an added range check that is likely to get optimised out due to the use of const-sized arrays anyway. Adds some missing `#[repr(transparent)]` decorators to `fuel-types`. The `FuelMnemonic` type provided unnecessary indirection between the user and the free function, and has been removed in favour of exposing the function directly from the crate root. --- fuel-crypto/src/lib.rs | 3 +- fuel-crypto/src/message.rs | 61 ++++++---------- fuel-crypto/src/mnemonic.rs | 26 +++---- fuel-crypto/src/public.rs | 108 ++++++---------------------- fuel-crypto/src/secret.rs | 89 ++++------------------- fuel-crypto/src/signature.rs | 66 +++++------------ fuel-crypto/tests/mnemonic.rs | 4 +- fuel-tx/src/transaction/id.rs | 3 +- fuel-tx/src/transaction/validity.rs | 12 ++-- fuel-types/src/types.rs | 2 + fuel-vm/src/interpreter/crypto.rs | 8 ++- 11 files changed, 98 insertions(+), 284 deletions(-) diff --git a/fuel-crypto/src/lib.rs b/fuel-crypto/src/lib.rs index 1e79e994c1..18f6f4adfd 100644 --- a/fuel-crypto/src/lib.rs +++ b/fuel-crypto/src/lib.rs @@ -40,7 +40,8 @@ pub use error::Error; pub use hasher::Hasher; pub use keystore::Keystore; pub use message::Message; -pub use mnemonic::FuelMnemonic; +#[cfg(all(feature = "std", feature = "random"))] +pub use mnemonic::generate_mnemonic_phrase; pub use public::PublicKey; pub use secret::SecretKey; pub use signature::Signature; diff --git a/fuel-crypto/src/message.rs b/fuel-crypto/src/message.rs index a2a441b1ee..a01108f1e3 100644 --- a/fuel-crypto/src/message.rs +++ b/fuel-crypto/src/message.rs @@ -1,9 +1,7 @@ use crate::Hasher; - -pub use fuel_types::Bytes32; - use core::fmt; use core::ops::Deref; +pub use fuel_types::Bytes32; /// Normalized signature message #[derive(Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -12,10 +10,11 @@ use core::ops::Deref; pub struct Message(Bytes32); impl Message { - /// Memory length of the type + /// Memory length of the type in bytes. pub const LEN: usize = Bytes32::LEN; - /// Normalize a message for signature + /// Normalize the given message by cryptographically hashing its content in + /// preparation for signing. pub fn new(message: M) -> Self where M: AsRef<[u8]>, @@ -23,43 +22,21 @@ impl Message { Self(Hasher::hash(message)) } - /// Add a conversion from arbitrary slices into owned + /// Construct a `Message` directly from its bytes. /// - /// # Safety - /// - /// There is no guarantee the provided bytes will be the product of a cryptographically secure - /// hash. Using insecure messages might compromise the security of the signature. - pub unsafe fn from_bytes_unchecked(bytes: [u8; Self::LEN]) -> Self { + /// This constructor expects the given bytes to be a valid, + /// cryptographically hashed message. No hashing is performed. + pub fn from_bytes(bytes: [u8; Self::LEN]) -> Self { Self(bytes.into()) } - /// Add a conversion from arbitrary slices into owned - /// - /// # Safety + /// Construct a `Message` reference directly from a reference to its bytes. /// - /// This function will not panic if the length of the slice is smaller than - /// `Self::LEN`. Instead, it will cause undefined behavior and read random - /// disowned bytes. - /// - /// This function extends the unsafety of [`Self::from_bytes_unchecked`]. - pub unsafe fn from_slice_unchecked(bytes: &[u8]) -> Self { - Self(Bytes32::from_slice_unchecked(bytes)) - } - - /// Copy-free reference cast - /// - /// # Safety - /// - /// Inputs smaller than `Self::LEN` will cause undefined behavior. - /// - /// This function extends the unsafety of [`Self::from_bytes_unchecked`]. - pub unsafe fn as_ref_unchecked(bytes: &[u8]) -> &Self { - // The interpreter will frequently make references to keys and values using - // logically checked slices. - // - // This function will avoid unnecessary copy to owned slices for the interpreter - // access - &*(bytes.as_ptr() as *const Self) + /// This constructor expects the given bytes to be a valid, + /// cryptographically hashed message. No hashing is performed. + pub fn from_bytes_ref(bytes: &[u8; Self::LEN]) -> &Self { + // TODO: Wrap this unsafe conversion safely in `fuel_types::Bytes32`. + unsafe { &*(bytes.as_ptr() as *const Self) } } } @@ -83,17 +60,23 @@ impl From for [u8; Message::LEN] { } } +impl From for Bytes32 { + fn from(s: Message) -> Self { + s.0 + } +} + impl From<&Hasher> for Message { fn from(hasher: &Hasher) -> Self { // Safety: `Hasher` is a cryptographic hash - unsafe { Self::from_bytes_unchecked(*hasher.digest()) } + Self::from_bytes(*hasher.digest()) } } impl From for Message { fn from(hasher: Hasher) -> Self { // Safety: `Hasher` is a cryptographic hash - unsafe { Self::from_bytes_unchecked(*hasher.finalize()) } + Self::from_bytes(*hasher.finalize()) } } diff --git a/fuel-crypto/src/mnemonic.rs b/fuel-crypto/src/mnemonic.rs index 3576c3cc75..5ddbda6e5f 100644 --- a/fuel-crypto/src/mnemonic.rs +++ b/fuel-crypto/src/mnemonic.rs @@ -1,21 +1,11 @@ -/// FuelMnemonic is a simple mnemonic phrase generator. -pub struct FuelMnemonic; +#![cfg(all(feature = "std", feature = "random"))] -#[cfg(all(feature = "std", feature = "random"))] -mod use_std { - use super::FuelMnemonic; - use crate::Error; - use coins_bip39::{English, Mnemonic}; +use crate::Error; +use coins_bip39::{English, Mnemonic}; +use rand::Rng; - pub type W = English; - - use rand::Rng; - - impl FuelMnemonic { - /// Generates a random mnemonic phrase given a random number generator and - /// the number of words to generate, `count`. - pub fn generate_mnemonic_phrase(rng: &mut R, count: usize) -> Result { - Ok(Mnemonic::::new_with_count(rng, count)?.to_phrase()?) - } - } +/// Generates a random mnemonic phrase given a random number generator and +/// the number of words to generate, `count`. +pub fn generate_mnemonic_phrase(rng: &mut R, count: usize) -> Result { + Ok(Mnemonic::::new_with_count(rng, count)?.to_phrase()?) } diff --git a/fuel-crypto/src/public.rs b/fuel-crypto/src/public.rs index 9d34f2db32..8d1fcbe504 100644 --- a/fuel-crypto/src/public.rs +++ b/fuel-crypto/src/public.rs @@ -1,9 +1,7 @@ -use crate::Hasher; - -use fuel_types::{Bytes32, Bytes64}; - +use crate::hasher::Hasher; use core::fmt; use core::ops::Deref; +use fuel_types::{Bytes32, Bytes64}; /// Asymmetric public key #[derive(Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -12,51 +10,18 @@ use core::ops::Deref; pub struct PublicKey(Bytes64); impl PublicKey { - /// Memory length of the type + /// Memory length of the type in bytes. pub const LEN: usize = Bytes64::LEN; - /// Copy-free reference cast + /// Construct a `PublicKey` directly from its bytes. /// - /// # Safety - /// - /// This function will not panic if the length of the slice is smaller than - /// `Self::LEN`. Instead, it will cause undefined behavior and read random - /// disowned bytes. - /// - /// There is no guarantee the provided bytes will fit the curve. - pub unsafe fn as_ref_unchecked(bytes: &[u8]) -> &Self { - // The interpreter will frequently make references to keys and values using - // logically checked slices. - // - // This function will save unnecessary copy to owned slices for the interpreter - // access - &*(bytes.as_ptr() as *const Self) - } - - /// Add a conversion from arbitrary slices into owned - /// - /// # Safety - /// - /// There is no guarantee the provided bytes will fit the curve. The curve - /// security can be checked with [`PublicKey::is_in_curve`]. - pub unsafe fn from_bytes_unchecked(bytes: [u8; Self::LEN]) -> Self { + /// This constructor expects the given bytes to be a valid public key, and + /// does not check whether the public key is within the curve. + fn from_bytes_unchecked(bytes: [u8; Self::LEN]) -> Self { Self(bytes.into()) } - /// Add a conversion from arbitrary slices into owned - /// - /// # Safety - /// - /// This function will not panic if the length of the slice is smaller than - /// `Self::LEN`. Instead, it will cause undefined behavior and read random - /// disowned bytes. - /// - /// There is no guarantee the provided bytes will fit the curve. - pub unsafe fn from_slice_unchecked(bytes: &[u8]) -> Self { - Self(Bytes64::from_slice_unchecked(bytes)) - } - - /// Hash of the public key + /// Cryptographic hash of the public key. pub fn hash(&self) -> Bytes32 { Hasher::hash(self.as_ref()) } @@ -117,52 +82,19 @@ mod use_std { use super::*; use crate::{Error, SecretKey}; - use secp256k1::{Error as Secp256k1Error, PublicKey as Secp256k1PublicKey, Secp256k1}; + use secp256k1::{ + constants::UNCOMPRESSED_PUBLIC_KEY_SIZE, Error as Secp256k1Error, PublicKey as Secp256k1PublicKey, Secp256k1, + }; use core::borrow::Borrow; use core::str; - const UNCOMPRESSED_PUBLIC_KEY_SIZE: usize = 65; - // Internal secp256k1 identifier for uncompressed point // // https://github.com/rust-bitcoin/rust-secp256k1/blob/ecb62612b57bf3aa8d8017d611d571f86bfdb5dd/secp256k1-sys/depend/secp256k1/include/secp256k1.h#L196 const SECP_UNCOMPRESSED_FLAG: u8 = 4; impl PublicKey { - /// Check if the provided slice represents a public key that is in the - /// curve. - /// - /// # Safety - /// - /// This function extends the unsafety of - /// [`PublicKey::as_ref_unchecked`]. - pub unsafe fn is_slice_in_curve_unchecked(slice: &[u8]) -> bool { - use secp256k1::ffi::{self, CPtr}; - - let public = Self::as_ref_unchecked(slice); - - let mut public_with_flag = [0u8; UNCOMPRESSED_PUBLIC_KEY_SIZE]; - - public_with_flag[1..].copy_from_slice(public.as_ref()); - - // Safety: FFI call - let curve = ffi::secp256k1_ec_pubkey_parse( - ffi::secp256k1_context_no_precomp, - &mut ffi::PublicKey::new(), - public_with_flag.as_c_ptr(), - UNCOMPRESSED_PUBLIC_KEY_SIZE, - ); - - curve == 1 - } - - /// Check if the secret key representation is in the curve. - pub fn is_in_curve(&self) -> bool { - // Safety: struct is guaranteed to reference itself with correct len - unsafe { Self::is_slice_in_curve_unchecked(self.as_ref()) } - } - pub(crate) fn from_secp(pk: &Secp256k1PublicKey) -> PublicKey { debug_assert_eq!( UNCOMPRESSED_PUBLIC_KEY_SIZE, @@ -174,10 +106,9 @@ mod use_std { debug_assert_eq!(SECP_UNCOMPRESSED_FLAG, pk[0]); // Ignore the first byte of the compression flag - let pk = &pk[1..]; + let bytes = <[u8; Self::LEN]>::try_from(&pk[1..]).expect("compile-time bounds-checks"); - // Safety: compile-time assertion of size correctness - unsafe { Self::from_slice_unchecked(pk) } + Self::from_bytes_unchecked(bytes) } pub(crate) fn _to_secp(&self) -> Result { @@ -197,9 +128,7 @@ mod use_std { type Error = Error; fn try_from(b: Bytes64) -> Result { - let public = PublicKey(b); - - public.is_in_curve().then_some(public).ok_or(Error::InvalidPublicKey) + public_key_bytes_valid(&b).map(|_| Self(b)) } } @@ -236,4 +165,13 @@ mod use_std { .and_then(PublicKey::try_from) } } + + /// Check if the public key byte representation is in the curve. + fn public_key_bytes_valid(bytes: &[u8; PublicKey::LEN]) -> Result<(), Error> { + let mut public_with_flag = [0u8; UNCOMPRESSED_PUBLIC_KEY_SIZE]; + public_with_flag[1..].copy_from_slice(bytes); + secp256k1::PublicKey::from_slice(&public_with_flag) + .map(|_| ()) + .map_err(|_| Error::InvalidPublicKey) + } } diff --git a/fuel-crypto/src/secret.rs b/fuel-crypto/src/secret.rs index 8ebe2cd450..3ddb7f9a87 100644 --- a/fuel-crypto/src/secret.rs +++ b/fuel-crypto/src/secret.rs @@ -15,44 +15,12 @@ impl SecretKey { /// Memory length of the type pub const LEN: usize = Bytes32::LEN; - /// Add a conversion from arbitrary slices into owned + /// Construct a `SecretKey` directly from its bytes. /// - /// # Safety - /// - /// There is no guarantee the provided bytes will fit the field. The field - /// security can be checked with [`SecretKey::is_in_field`]. - pub unsafe fn from_bytes_unchecked(bytes: [u8; Self::LEN]) -> Self { + /// This constructor expects the given bytes to be a valid secret key. Validity is unchecked. + fn from_bytes_unchecked(bytes: [u8; Self::LEN]) -> Self { Self(bytes.into()) } - - /// Add a conversion from arbitrary slices into owned - /// - /// # Safety - /// - /// This function will not panic if the length of the slice is smaller than - /// `Self::LEN`. Instead, it will cause undefined behavior and read random - /// disowned bytes. - /// - /// There is no guarantee the provided bytes will fit the field. - pub unsafe fn from_slice_unchecked(bytes: &[u8]) -> Self { - Self(Bytes32::from_slice_unchecked(bytes)) - } - - /// Copy-free reference cast - /// - /// There is no guarantee the provided bytes will fit the field. - /// - /// # Safety - /// - /// Inputs smaller than `Self::LEN` will cause undefined behavior. - pub unsafe fn as_ref_unchecked(bytes: &[u8]) -> &Self { - // The interpreter will frequently make references to keys and values using - // logically checked slices. - // - // This function will avoid unnecessary copy to owned slices for the interpreter - // access - &*(bytes.as_ptr() as *const Self) - } } impl Deref for SecretKey { @@ -133,23 +101,13 @@ mod use_std { // // We don't call `Secp256k1SecretKey::new` here because the `rand` requirements // are outdated and inconsistent. - - use secp256k1::ffi::{self, CPtr}; - let mut secret = Bytes32::zeroed(); - loop { rng.fill(secret.as_mut()); - - // Safety: FFI call - let overflow = - unsafe { ffi::secp256k1_ec_seckey_verify(ffi::secp256k1_context_no_precomp, secret.as_c_ptr()) }; - - if overflow != 0 { + if secret_key_bytes_valid(&secret).is_ok() { break; } } - Self(secret) } @@ -168,34 +126,8 @@ mod use_std { pub fn new_from_mnemonic(d: DerivationPath, m: Mnemonic) -> Result { let derived_priv_key = m.derive_key(d, None)?; let key: &coins_bip32::prelude::SigningKey = derived_priv_key.as_ref(); - - // Safety: this slice will always be of the expected length (`Bytes32`) - // because it will be a `Secp256k` secret key, coming from - // `coins_bip32::prelude::SigningKey`, which is a 256-bit (32-byte) scalar. - Ok(unsafe { SecretKey::from_slice_unchecked(key.to_bytes().as_ref()) }) - } - - /// Check if the provided slice represents a scalar that fits the field. - /// - /// # Safety - /// - /// This function extends the unsafety of - /// [`SecretKey::as_ref_unchecked`]. - pub unsafe fn is_slice_in_field_unchecked(slice: &[u8]) -> bool { - use secp256k1::ffi::{self, CPtr}; - - let secret = Self::as_ref_unchecked(slice); - - // Safety: FFI call - let overflow = ffi::secp256k1_ec_seckey_verify(ffi::secp256k1_context_no_precomp, secret.as_c_ptr()); - - overflow != 0 - } - - /// Check if the secret key representation fits the scalar field. - pub fn is_in_field(&self) -> bool { - // Safety: struct is guaranteed to reference itself with correct len - unsafe { Self::is_slice_in_field_unchecked(self.as_ref()) } + let bytes: [u8; Self::LEN] = key.to_bytes().into(); + Ok(SecretKey::from_bytes_unchecked(bytes)) } /// Return the curve representation of this secret. @@ -211,9 +143,7 @@ mod use_std { type Error = Error; fn try_from(b: Bytes32) -> Result { - let secret = SecretKey(b); - - secret.is_in_field().then_some(secret).ok_or(Error::InvalidSecretKey) + secret_key_bytes_valid(&b).map(|_| Self(b)) } } @@ -260,4 +190,9 @@ mod use_std { SecretKey::random(rng) } } + + /// Check if the secret key byte representation is within the curve. + fn secret_key_bytes_valid(bytes: &[u8; SecretKey::LEN]) -> Result<(), Error> { + secp256k1::SecretKey::from_slice(bytes).map(|_| ()).map_err(Into::into) + } } diff --git a/fuel-crypto/src/signature.rs b/fuel-crypto/src/signature.rs index 3282298ce9..5a539a1632 100644 --- a/fuel-crypto/src/signature.rs +++ b/fuel-crypto/src/signature.rs @@ -12,49 +12,22 @@ use core::{fmt, str}; pub struct Signature(Bytes64); impl Signature { - /// Memory length of the type + /// Memory length of the type in bytes. pub const LEN: usize = Bytes64::LEN; - /// Add a conversion from arbitrary slices into owned + /// Construct a `Signature` directly from its bytes. /// - /// # Safety - /// - /// There is no guarantee the provided bytes will be a valid signature. Internally, some FFI - /// calls to `secp256k1` are performed and we might have undefined behavior in case the bytes - /// are not canonically encoded to a valid `secp256k1` signature. - pub unsafe fn from_bytes_unchecked(bytes: [u8; Self::LEN]) -> Self { + /// This constructor expects the given bytes to be a valid signature. No signing is performed. + pub fn from_bytes(bytes: [u8; Self::LEN]) -> Self { Self(bytes.into()) } - /// Add a conversion from arbitrary slices into owned - /// - /// # Safety - /// - /// This function will not panic if the length of the slice is smaller than - /// `Self::LEN`. Instead, it will cause undefined behavior and read random - /// disowned bytes. - /// - /// There is no guarantee the provided bytes will be a valid signature. Internally, some FFI - /// calls to `secp256k1` are performed and we might have undefined behavior in case the bytes - /// are not canonically encoded to a valid `secp256k1` signature. - pub unsafe fn from_slice_unchecked(bytes: &[u8]) -> Self { - Self(Bytes64::from_slice_unchecked(bytes)) - } - - /// Copy-free reference cast - /// - /// There is no guarantee the provided bytes will fit the field. - /// - /// # Safety + /// Construct a `Signature` reference directly from a reference to its bytes. /// - /// Inputs smaller than `Self::LEN` will cause undefined behavior. - pub unsafe fn as_ref_unchecked(bytes: &[u8]) -> &Self { - // The interpreter will frequently make references to keys and values using - // logically checked slices. - // - // This function will avoid unnecessary copy to owned slices for the interpreter - // access - &*(bytes.as_ptr() as *const Self) + /// This constructor expects the given bytes to be a valid signature. No signing is performed. + pub fn from_bytes_ref(bytes: &[u8; Self::LEN]) -> &Self { + // TODO: Wrap this unsafe conversion safely in `fuel_types::Bytes64`. + unsafe { &*(bytes.as_ptr() as *const Self) } } } @@ -78,12 +51,6 @@ impl AsMut<[u8]> for Signature { } } -impl From for [u8; Signature::LEN] { - fn from(salt: Signature) -> [u8; Signature::LEN] { - salt.0.into() - } -} - impl fmt::LowerHex for Signature { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.0.fmt(f) @@ -108,9 +75,9 @@ impl fmt::Display for Signature { } } -impl From for Signature { - fn from(b: Bytes64) -> Self { - Self(b) +impl From for [u8; Signature::LEN] { + fn from(salt: Signature) -> [u8; Signature::LEN] { + salt.0.into() } } @@ -122,10 +89,13 @@ impl From for Bytes64 { impl str::FromStr for Signature { type Err = Error; + /// Parse a `Signature` directly from its bytes encoded as hex in a string. + /// + /// This constructor does not perform any signing. fn from_str(s: &str) -> Result { Bytes64::from_str(s) .map_err(|_| Error::InvalidSignature) - .map(|s| s.into()) + .map(|s| Self::from_bytes(s.into())) } } @@ -170,9 +140,7 @@ mod use_std { let v = v.to_i32(); signature[32] |= (v << 7) as u8; - - // Safety: the security of this call reflects the security of secp256k1 FFI - unsafe { Signature::from_bytes_unchecked(signature) } + Signature::from_bytes(signature) } /// Truncate the recovery id from the signature, producing a valid `secp256k1` diff --git a/fuel-crypto/tests/mnemonic.rs b/fuel-crypto/tests/mnemonic.rs index d385b7a9b7..4a75ca3f17 100644 --- a/fuel-crypto/tests/mnemonic.rs +++ b/fuel-crypto/tests/mnemonic.rs @@ -2,7 +2,7 @@ use std::str::FromStr; use coins_bip32::path::DerivationPath; use coins_bip39::{English, Mnemonic}; -use fuel_crypto::{FuelMnemonic, SecretKey}; +use fuel_crypto::SecretKey; type W = English; @@ -41,7 +41,7 @@ fn random_mnemonic_phrase() { // create rng let mut rng = rand::thread_rng(); - let phrase = FuelMnemonic::generate_mnemonic_phrase(&mut rng, 12).expect("failed to generate mnemonic phrase"); + let phrase = fuel_crypto::generate_mnemonic_phrase(&mut rng, 12).expect("failed to generate mnemonic phrase"); let _secret = SecretKey::new_from_mnemonic_phrase_with_path(&phrase, "m/44'/60'/0'/0/0") .expect("failed to create secret key from mnemonic phrase"); diff --git a/fuel-tx/src/transaction/id.rs b/fuel-tx/src/transaction/id.rs index 1173f40c28..fee5d6872d 100644 --- a/fuel-tx/src/transaction/id.rs +++ b/fuel-tx/src/transaction/id.rs @@ -40,8 +40,7 @@ where let pk = Input::owner(&pk); let id = self.id(); - // Safety: checked length - let message = unsafe { Message::as_ref_unchecked(id.as_ref()) }; + let message = Message::from_bytes_ref(&id); let signature = Signature::sign(secret, message); diff --git a/fuel-tx/src/transaction/validity.rs b/fuel-tx/src/transaction/validity.rs index 4ff1743b23..b8e4b28ffa 100644 --- a/fuel-tx/src/transaction/validity.rs +++ b/fuel-tx/src/transaction/validity.rs @@ -48,15 +48,11 @@ impl Input { .ok_or(CheckError::InputWitnessIndexBounds { index })? .as_ref(); - if witness.len() != Signature::LEN { - return Err(CheckError::InputInvalidSignature { index }); - } - - // Safety: checked length - let signature = unsafe { Signature::as_ref_unchecked(witness) }; + let bytes = <[u8; Signature::LEN]>::try_from(witness) + .map_err(|_| CheckError::InputInvalidSignature { index })?; + let signature = Signature::from_bytes(bytes); - // Safety: checked length - let message = unsafe { Message::as_ref_unchecked(txhash.as_ref()) }; + let message = Message::from_bytes_ref(txhash); let pk = signature .recover(message) diff --git a/fuel-types/src/types.rs b/fuel-types/src/types.rs index df9af79702..c8fba4f10b 100644 --- a/fuel-types/src/types.rs +++ b/fuel-types/src/types.rs @@ -26,6 +26,7 @@ macro_rules! key { ($i:ident, $s:expr) => { #[derive(Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] /// FuelVM atomic type. + #[repr(transparent)] pub struct $i([u8; $s]); key_methods!($i, $s); @@ -43,6 +44,7 @@ macro_rules! key_with_big_array { ($i:ident, $s:expr) => { #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] /// FuelVM atomic type. + #[repr(transparent)] pub struct $i([u8; $s]); key_methods!($i, $s); diff --git a/fuel-vm/src/interpreter/crypto.rs b/fuel-vm/src/interpreter/crypto.rs index feb347c06f..4bde32f33d 100644 --- a/fuel-vm/src/interpreter/crypto.rs +++ b/fuel-vm/src/interpreter/crypto.rs @@ -22,11 +22,13 @@ where return Err(PanicReason::MemoryOverflow.into()); } + // TODO: These casts may overflow/truncate on 32-bit? let (a, b, bx, c, cx) = (a as usize, b as usize, bx as usize, c as usize, cx as usize); - // Safety: memory bounds are checked - let signature = unsafe { Signature::as_ref_unchecked(&self.memory[b..bx]) }; - let message = unsafe { Message::as_ref_unchecked(&self.memory[c..cx]) }; + let sig_bytes = <&_>::try_from(&self.memory[b..bx]).expect("memory bounds checked"); + let msg_bytes = <&_>::try_from(&self.memory[c..cx]).expect("memory bounds checked"); + let signature = Signature::from_bytes_ref(sig_bytes); + let message = Message::from_bytes_ref(msg_bytes); match signature.recover(message) { Ok(pub_key) => { From c1ec2c8b994b776748d499099cb5aa5776e8678b Mon Sep 17 00:00:00 2001 From: mitchmindtree Date: Sat, 11 Feb 2023 03:43:08 +1100 Subject: [PATCH 2/3] Re-add but deprecate Signature/Message from_bytes_unchecked for best-effort back-compat --- fuel-crypto/src/message.rs | 6 ++++++ fuel-crypto/src/signature.rs | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/fuel-crypto/src/message.rs b/fuel-crypto/src/message.rs index a01108f1e3..fab450ac0a 100644 --- a/fuel-crypto/src/message.rs +++ b/fuel-crypto/src/message.rs @@ -38,6 +38,12 @@ impl Message { // TODO: Wrap this unsafe conversion safely in `fuel_types::Bytes32`. unsafe { &*(bytes.as_ptr() as *const Self) } } + + /// Kept temporarily for backwards compatibility. + #[deprecated = "Use `Message::from_bytes` instead"] + pub fn from_bytes_unchecked(bytes: [u8; Self::LEN]) -> Self { + Self::from_bytes(bytes) + } } impl Deref for Message { diff --git a/fuel-crypto/src/signature.rs b/fuel-crypto/src/signature.rs index 5a539a1632..3e449d0595 100644 --- a/fuel-crypto/src/signature.rs +++ b/fuel-crypto/src/signature.rs @@ -29,6 +29,12 @@ impl Signature { // TODO: Wrap this unsafe conversion safely in `fuel_types::Bytes64`. unsafe { &*(bytes.as_ptr() as *const Self) } } + + /// Kept temporarily for backwards compatibility. + #[deprecated = "Use `Signature::from_bytes` instead"] + pub fn from_bytes_unchecked(bytes: [u8; Self::LEN]) -> Self { + Self::from_bytes(bytes) + } } impl Deref for Signature { From 0db0fa85eae609c595faaeef8e1dad1de6ed0dbd Mon Sep 17 00:00:00 2001 From: mitchmindtree Date: Tue, 14 Feb 2023 10:10:47 +1100 Subject: [PATCH 3/3] Update secp256k1 dep from 0.24 to 0.26 --- fuel-crypto/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fuel-crypto/Cargo.toml b/fuel-crypto/Cargo.toml index 2b06a15063..e46e985c48 100644 --- a/fuel-crypto/Cargo.toml +++ b/fuel-crypto/Cargo.toml @@ -17,7 +17,7 @@ coins-bip39 = { version = "0.7", default-features = false, optional = true } fuel-types = { workspace = true, default-features = false } lazy_static = { version = "1.4", optional = true } rand = { version = "0.8", default-features = false, optional = true } -secp256k1 = { version = "0.24", default-features = false, features = ["recovery"], optional = true } +secp256k1 = { version = "0.26", default-features = false, features = ["recovery"], optional = true } serde = { version = "1.0", default-features = false, features = ["derive"], optional = true } sha2 = { version = "0.10", default-features = false } zeroize = { version = "1.5", features = ["derive"] }