From 839c8de16a8db815fe9b33a768f26f0983b172a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 19 Dec 2022 19:58:25 +0100 Subject: [PATCH] ed25519: Don't panic for invalid signature (#12965) * ed25519: Don't panic for invalid signature We should not panic for an invalid signature when the `UseDalekExt` is given. * Update Cargo.toml * Update primitives/io/Cargo.toml --- Cargo.lock | 5 +++-- primitives/io/Cargo.toml | 5 +++++ primitives/io/src/lib.rs | 26 ++++++++++++++++++++++---- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 046e5991ba762..8412781c682e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1566,9 +1566,9 @@ dependencies = [ [[package]] name = "ed25519" -version = "1.0.3" +version = "1.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "37c66a534cbb46ab4ea03477eae19d5c22c01da8258030280b7bd9d8433fb6ef" +checksum = "1e9c280362032ea4203659fc489832d0204ef09f247a0506f170dafcac08c369" dependencies = [ "signature", ] @@ -9181,6 +9181,7 @@ name = "sp-io" version = "7.0.0" dependencies = [ "bytes", + "ed25519", "ed25519-dalek", "futures", "libsecp256k1", diff --git a/primitives/io/Cargo.toml b/primitives/io/Cargo.toml index 57dc3604de73f..d83c82c02227b 100644 --- a/primitives/io/Cargo.toml +++ b/primitives/io/Cargo.toml @@ -31,7 +31,11 @@ futures = { version = "0.3.21", features = ["thread-pool"], optional = true } secp256k1 = { version = "0.24.0", features = ["recovery", "global-context"], optional = true } tracing = { version = "0.1.29", default-features = false } tracing-core = { version = "0.1.28", default-features = false} + +# Required for backwards compatibility reason, but only used for verifying when `UseDalekExt` is set. ed25519-dalek = { version = "1.0.1", default-features = false, optional = true } +# Force the usage of ed25519, this is being used in `ed25519-dalek`. +ed25519 = { version = "1.5.2", optional = true } [features] default = ["std"] @@ -53,6 +57,7 @@ std = [ "log", "futures", "ed25519-dalek", + "ed25519", ] with-tracing = [ diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 600d76b3b4300..bb06c00ee2c6f 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -783,13 +783,13 @@ pub trait Crypto { { use ed25519_dalek::Verifier; - let public_key = if let Ok(vk) = ed25519_dalek::PublicKey::from_bytes(&pub_key.0) { - vk - } else { + let Ok(public_key) = ed25519_dalek::PublicKey::from_bytes(&pub_key.0) else { return false }; - let sig = ed25519_dalek::Signature::from(sig.0); + let Ok(sig) = ed25519_dalek::Signature::from_bytes(&sig.0) else { + return false + }; public_key.verify(msg, &sig).is_ok() } else { @@ -1946,4 +1946,22 @@ mod tests { assert!(crypto::ed25519_verify(&zero_ed_sig(), &Vec::new(), &zero_ed_pub())); }) } + + #[test] + fn dalek_should_not_panic_on_invalid_signature() { + let mut ext = BasicExternalities::default(); + ext.register_extension(UseDalekExt::default()); + + ext.execute_with(|| { + let mut bytes = [0u8; 64]; + // Make it invalid + bytes[63] = 0b1110_0000; + + assert!(!crypto::ed25519_verify( + &ed25519::Signature::from_raw(bytes), + &Vec::new(), + &zero_ed_pub() + )); + }); + } }