Skip to content

Commit

Permalink
ed25519: Don't panic for invalid signature (paritytech#12965)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
bkchr authored and ltfschoen committed Feb 22, 2023
1 parent a6cc8d4 commit 839c8de
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions primitives/io/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -53,6 +57,7 @@ std = [
"log",
"futures",
"ed25519-dalek",
"ed25519",
]

with-tracing = [
Expand Down
26 changes: 22 additions & 4 deletions primitives/io/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
));
});
}
}

0 comments on commit 839c8de

Please sign in to comment.