From d7b56198e9df2309f1594e2cd6b0c73971dcc1f5 Mon Sep 17 00:00:00 2001 From: Trevor Arjeski Date: Wed, 14 Jul 2021 09:14:44 +0300 Subject: [PATCH] RUSTSEC-2021-0076 bump libsecp256k1 libsecp256k1 allows overflowing signatures https://rustsec.org/advisories/RUSTSEC-2021-0076 Changes were made to conform to libsecp256k1 version differences. Closes #9356 --- Cargo.lock | 69 +++++++++++++++++++++++++++++--- primitives/core/Cargo.toml | 2 +- primitives/core/src/ecdsa.rs | 76 +++++++++++++----------------------- 3 files changed, 93 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bd6bcff83fd57..ca5d42d5370bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2476,6 +2476,17 @@ dependencies = [ "hmac 0.7.1", ] +[[package]] +name = "hmac-drbg" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "17ea0a1394df5b6574da6e0c1ade9e78868c9fb0a4e5ef4428e32da4676b85b1" +dependencies = [ + "digest 0.9.0", + "generic-array 0.14.4", + "hmac 0.8.1", +] + [[package]] name = "honggfuzz" version = "0.5.54" @@ -3236,7 +3247,7 @@ dependencies = [ "futures 0.3.15", "futures-timer 3.0.2", "lazy_static", - "libsecp256k1", + "libsecp256k1 0.3.5", "log", "multihash", "multistream-select", @@ -3638,13 +3649,61 @@ dependencies = [ "arrayref", "crunchy", "digest 0.8.1", - "hmac-drbg", + "hmac-drbg 0.2.0", "rand 0.7.3", "sha2 0.8.2", "subtle 2.4.0", "typenum", ] +[[package]] +name = "libsecp256k1" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c9d220bc1feda2ac231cb78c3d26f27676b8cf82c96971f7aeef3d0cf2797c73" +dependencies = [ + "arrayref", + "base64 0.12.3", + "digest 0.9.0", + "hmac-drbg 0.3.0", + "libsecp256k1-core", + "libsecp256k1-gen-ecmult", + "libsecp256k1-gen-genmult", + "rand 0.7.3", + "serde", + "sha2 0.9.3", + "typenum", +] + +[[package]] +name = "libsecp256k1-core" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d0f6ab710cec28cef759c5f18671a27dae2a5f952cdaaee1d8e2908cb2478a80" +dependencies = [ + "crunchy", + "digest 0.9.0", + "subtle 2.4.0", +] + +[[package]] +name = "libsecp256k1-gen-ecmult" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ccab96b584d38fac86a83f07e659f0deafd0253dc096dab5a36d53efe653c5c3" +dependencies = [ + "libsecp256k1-core", +] + +[[package]] +name = "libsecp256k1-gen-genmult" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67abfe149395e3aa1c48a2beb32b068e2334402df8181f818d3aee2b304c4f5d" +dependencies = [ + "libsecp256k1-core", +] + [[package]] name = "libz-sys" version = "1.1.2" @@ -7514,7 +7573,7 @@ dependencies = [ "derive_more", "hex-literal", "lazy_static", - "libsecp256k1", + "libsecp256k1 0.3.5", "log", "parity-scale-codec", "parity-wasm 0.42.2", @@ -8952,7 +9011,7 @@ dependencies = [ "hex-literal", "impl-serde", "lazy_static", - "libsecp256k1", + "libsecp256k1 0.6.0", "log", "merlin", "num-traits", @@ -9047,7 +9106,7 @@ version = "4.0.0-dev" dependencies = [ "futures 0.3.15", "hash-db", - "libsecp256k1", + "libsecp256k1 0.3.5", "log", "parity-scale-codec", "parking_lot 0.11.1", diff --git a/primitives/core/Cargo.toml b/primitives/core/Cargo.toml index 711fcc37e8556..eff3bec2670ab 100644 --- a/primitives/core/Cargo.toml +++ b/primitives/core/Cargo.toml @@ -49,7 +49,7 @@ schnorrkel = { version = "0.9.1", features = ["preaudit_deprecated", "u64_backen sha2 = { version = "0.9.2", default-features = false, optional = true } hex = { version = "0.4", default-features = false, optional = true } twox-hash = { version = "1.5.0", default-features = false, optional = true } -libsecp256k1 = { version = "0.3.2", default-features = false, features = ["hmac"], optional = true } +libsecp256k1 = { version = "0.6", default-features = false, features = ["hmac", "static-context"], optional = true } merlin = { version = "2.0", default-features = false, optional = true } sp-runtime-interface = { version = "4.0.0-dev", default-features = false, path = "../runtime-interface" } diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index b4c4bda17acba..33ed1615e52fe 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -40,12 +40,7 @@ use bip39::{Language, Mnemonic, MnemonicType}; #[cfg(feature = "full_crypto")] use core::convert::{TryFrom, TryInto}; #[cfg(feature = "full_crypto")] -use secp256k1::{PublicKey, SecretKey}; -#[cfg(feature = "std")] -use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; -use sp_runtime_interface::pass_by::PassByInner; -#[cfg(feature = "std")] -use substrate_bip39::seed_from_entropy; +use libsecp256k1::{PublicKey, SecretKey}; /// An identifier used to match public keys against ecdsa keys pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecds"); @@ -108,7 +103,7 @@ impl Public { /// This will convert the full public key into the compressed format. #[cfg(feature = "std")] pub fn from_full(full: &[u8]) -> Result { - secp256k1::PublicKey::parse_slice(full, None) + libsecp256k1::PublicKey::parse_slice(full, None) .map(|k| k.serialize_compressed()) .map(Self) .map_err(|_| ()) @@ -364,9 +359,9 @@ impl Signature { /// Recover the public key from this signature and a message. #[cfg(feature = "full_crypto")] pub fn recover>(&self, message: M) -> Option { - let message = secp256k1::Message::parse(&blake2_256(message.as_ref())); + let message = libsecp256k1::Message::parse(&blake2_256(message.as_ref())); let sig: (_, _) = self.try_into().ok()?; - secp256k1::recover(&message, &sig.0, &sig.1) + libsecp256k1::recover(&message, &sig.0, &sig.1) .ok() .map(|recovered| Public(recovered.serialize_compressed())) } @@ -374,19 +369,19 @@ impl Signature { /// Recover the public key from this signature and a pre-hashed message. #[cfg(feature = "full_crypto")] pub fn recover_prehashed(&self, message: &[u8; 32]) -> Option { - let message = secp256k1::Message::parse(message); + let message = libsecp256k1::Message::parse(message); let sig: (_, _) = self.try_into().ok()?; - secp256k1::recover(&message, &sig.0, &sig.1) + libsecp256k1::recover(&message, &sig.0, &sig.1) .ok() .map(|key| Public(key.serialize_compressed())) } } #[cfg(feature = "full_crypto")] -impl From<(secp256k1::Signature, secp256k1::RecoveryId)> for Signature { - fn from(x: (secp256k1::Signature, secp256k1::RecoveryId)) -> Signature { +impl From<(libsecp256k1::Signature, libsecp256k1::RecoveryId)> for Signature { + fn from(x: (libsecp256k1::Signature, libsecp256k1::RecoveryId)) -> Signature { let mut r = Self::default(); r.0[0..64].copy_from_slice(&x.0.serialize()[..]); r.0[64] = x.1.serialize(); @@ -395,14 +390,12 @@ impl From<(secp256k1::Signature, secp256k1::RecoveryId)> for Signature { } #[cfg(feature = "full_crypto")] -impl<'a> TryFrom<&'a Signature> for (secp256k1::Signature, secp256k1::RecoveryId) { +impl<'a> TryFrom<&'a Signature> for (libsecp256k1::Signature, libsecp256k1::RecoveryId) { type Error = (); - fn try_from( - x: &'a Signature, - ) -> Result<(secp256k1::Signature, secp256k1::RecoveryId), Self::Error> { + fn try_from(x: &'a Signature) -> Result<(libsecp256k1::Signature, libsecp256k1::RecoveryId), Self::Error> { Ok(( - secp256k1::Signature::parse_slice(&x.0[0..64]).expect("hardcoded to 64 bytes; qed"), - secp256k1::RecoveryId::parse(x.0[64]).map_err(|_| ())?, + libsecp256k1::Signature::parse_standard_slice(&x.0[0..64]).expect("hardcoded to 64 bytes; qed"), + libsecp256k1::RecoveryId::parse(x.0[64]).map_err(|_| ())?, )) } } @@ -510,18 +503,15 @@ impl TraitPair for Pair { /// Sign a message. fn sign(&self, message: &[u8]) -> Signature { - let message = secp256k1::Message::parse(&blake2_256(message)); - secp256k1::sign(&message, &self.secret).into() + let message = libsecp256k1::Message::parse(&blake2_256(message)); + libsecp256k1::sign(&message, &self.secret).into() } /// Verify a signature on a message. Returns true if the signature is good. fn verify>(sig: &Self::Signature, message: M, pubkey: &Self::Public) -> bool { - let message = secp256k1::Message::parse(&blake2_256(message.as_ref())); - let sig: (_, _) = match sig.try_into() { - Ok(x) => x, - _ => return false, - }; - match secp256k1::recover(&message, &sig.0, &sig.1) { + let message = libsecp256k1::Message::parse(&blake2_256(message.as_ref())); + let sig: (_, _) = match sig.try_into() { Ok(x) => x, _ => return false }; + match libsecp256k1::recover(&message, &sig.0, &sig.1) { Ok(actual) => pubkey.0[..] == actual.serialize_compressed()[..], _ => false, } @@ -532,19 +522,11 @@ impl TraitPair for Pair { /// This doesn't use the type system to ensure that `sig` and `pubkey` are the correct /// size. Use it only if you're coming from byte buffers and need the speed. fn verify_weak, M: AsRef<[u8]>>(sig: &[u8], message: M, pubkey: P) -> bool { - let message = secp256k1::Message::parse(&blake2_256(message.as_ref())); - if sig.len() != 65 { - return false - } - let ri = match secp256k1::RecoveryId::parse(sig[64]) { - Ok(x) => x, - _ => return false, - }; - let sig = match secp256k1::Signature::parse_slice(&sig[0..64]) { - Ok(x) => x, - _ => return false, - }; - match secp256k1::recover(&message, &sig, &ri) { + let message = libsecp256k1::Message::parse(&blake2_256(message.as_ref())); + if sig.len() != 65 { return false } + let ri = match libsecp256k1::RecoveryId::parse(sig[64]) { Ok(x) => x, _ => return false }; + let sig = match libsecp256k1::Signature::parse_standard_slice(&sig[0..64]) { Ok(x) => x, _ => return false }; + match libsecp256k1::recover(&message, &sig, &ri) { Ok(actual) => pubkey.as_ref() == &actual.serialize()[1..], _ => false, } @@ -577,21 +559,21 @@ impl Pair { /// Sign a pre-hashed message pub fn sign_prehashed(&self, message: &[u8; 32]) -> Signature { - let message = secp256k1::Message::parse(message); - secp256k1::sign(&message, &self.secret).into() + let message = libsecp256k1::Message::parse(message); + libsecp256k1::sign(&message, &self.secret).into() } /// Verify a signature on a pre-hashed message. Return `true` if the signature is valid /// and thus matches the given `public` key. pub fn verify_prehashed(sig: &Signature, message: &[u8; 32], public: &Public) -> bool { - let message = secp256k1::Message::parse(message); + let message = libsecp256k1::Message::parse(message); let sig: (_, _) = match sig.try_into() { Ok(x) => x, _ => return false, }; - match secp256k1::recover(&message, &sig.0, &sig.1) { + match libsecp256k1::recover(&message, &sig.0, &sig.1) { Ok(actual) => public.0[..] == actual.serialize_compressed()[..], _ => false, } @@ -839,8 +821,7 @@ mod test { // `msg` shouldn't be mangled let msg = [0u8; 32]; let sig1 = pair.sign_prehashed(&msg); - let sig2: Signature = - secp256k1::sign(&secp256k1::Message::parse(&msg), &pair.secret).into(); + let sig2: Signature = libsecp256k1::sign(&libsecp256k1::Message::parse(&msg), &pair.secret).into(); assert_eq!(sig1, sig2); @@ -852,8 +833,7 @@ mod test { // using pre-hashed `msg` works let msg = keccak_256(b"this should be hashed"); let sig1 = pair.sign_prehashed(&msg); - let sig2: Signature = - secp256k1::sign(&secp256k1::Message::parse(&msg), &pair.secret).into(); + let sig2: Signature = libsecp256k1::sign(&libsecp256k1::Message::parse(&msg), &pair.secret).into(); assert_eq!(sig1, sig2); }