From a52b237694c229f806becf115b69716bf6ec9987 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Tue, 22 Oct 2024 17:41:34 +0400 Subject: [PATCH 1/4] feat: check signature s during recovery --- crates/eip7702/Cargo.toml | 2 ++ crates/eip7702/src/auth_list.rs | 13 +++++++++++-- crates/eip7702/src/constants.rs | 6 ++++++ crates/eip7702/src/error.rs | 12 ++++++++++++ crates/eip7702/src/lib.rs | 3 +++ 5 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 crates/eip7702/src/error.rs diff --git a/crates/eip7702/Cargo.toml b/crates/eip7702/Cargo.toml index 3623b14..190d6bc 100644 --- a/crates/eip7702/Cargo.toml +++ b/crates/eip7702/Cargo.toml @@ -32,6 +32,8 @@ arbitrary = { workspace = true, features = ["derive"], optional = true } k256 = { workspace = true, optional = true } rand = { workspace = true, optional = true } +derive_more = "1" + [dev-dependencies] bincode = "1.3" rand = "0.8" diff --git a/crates/eip7702/src/auth_list.rs b/crates/eip7702/src/auth_list.rs index 02f0636..d44f47e 100644 --- a/crates/eip7702/src/auth_list.rs +++ b/crates/eip7702/src/auth_list.rs @@ -127,6 +127,9 @@ impl SignedAuthorization { /// Gets the `signature` for the authorization. Returns [`SignatureError`] if signature could /// not be constructed from vrs values. + /// + /// Note that this signature might still be invalid for recovery as it might have `s` value + /// greater than [`SECP256K1N_HALF`]. pub fn signature(&self) -> Result { if self.y_parity() <= 1 { Ok(Signature::new(self.r, self.s, Parity::Parity(self.y_parity() == 1))) @@ -228,8 +231,14 @@ impl SignedAuthorization { /// # Note /// /// Implementers should check that the authority has no code. - pub fn recover_authority(&self) -> Result { - self.signature()?.recover_address_from_prehash(&self.inner.signature_hash()) + pub fn recover_authority(&self) -> Result { + let signature = self.signature()?; + + if signature.s() > crate::constants::SECP256K1N_HALF { + return Err(crate::error::Eip7702Error::InvalidSValue(signature.s())); + } + + Ok(signature.recover_address_from_prehash(&self.inner.signature_hash())?) } /// Recover the authority and transform the signed authorization into a diff --git a/crates/eip7702/src/constants.rs b/crates/eip7702/src/constants.rs index b573e35..b2297ae 100644 --- a/crates/eip7702/src/constants.rs +++ b/crates/eip7702/src/constants.rs @@ -1,6 +1,7 @@ //! [EIP-7702] constants. //! //! [EIP-7702]: https://eips.ethereum.org/EIPS/eip-7702 +use alloy_primitives::{uint, U256}; /// Identifier for EIP7702's set code transaction. /// @@ -23,3 +24,8 @@ pub const PER_AUTH_BASE_COST: u64 = 12500; /// /// See also [EIP-7702](https://eips.ethereum.org/EIPS/eip-7702). pub const PER_EMPTY_ACCOUNT_COST: u64 = 25000; + +/// The order of the secp256k1 curve, divided by two. Signatures that should be checked according +/// to EIP-2 should have an S value less than or equal to this. +pub const SECP256K1N_HALF: U256 = + uint!(57896044618658097711785492504343953926418782139537452191302581570759080747168_U256); diff --git a/crates/eip7702/src/error.rs b/crates/eip7702/src/error.rs new file mode 100644 index 0000000..381257b --- /dev/null +++ b/crates/eip7702/src/error.rs @@ -0,0 +1,12 @@ +use alloy_primitives::U256; + +/// EIP-7702 error. +#[derive(Debug, derive_more::Display, derive_more::From)] +pub enum Eip7702Error { + /// Invalid signature `s` value. + #[display("invalid signature `s` value: {_0}")] + InvalidSValue(U256), + /// Signature error. + #[from] + Signature(alloy_primitives::SignatureError), +} diff --git a/crates/eip7702/src/lib.rs b/crates/eip7702/src/lib.rs index eeff14a..8655f48 100644 --- a/crates/eip7702/src/lib.rs +++ b/crates/eip7702/src/lib.rs @@ -12,6 +12,9 @@ pub use auth_list::*; pub mod constants; +mod error; +pub use error::Eip7702Error; + /// Bincode-compatible serde implementations for EIP-7702 types. /// /// `bincode` crate doesn't work with `#[serde(flatten)]` attribute, but some of the EIP-7702 types From e3b0080922d4db57533a3cd5c0e277de38a8b6ab Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Tue, 22 Oct 2024 17:43:22 +0400 Subject: [PATCH 2/4] error impl --- crates/eip7702/src/error.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/eip7702/src/error.rs b/crates/eip7702/src/error.rs index 381257b..5fca484 100644 --- a/crates/eip7702/src/error.rs +++ b/crates/eip7702/src/error.rs @@ -10,3 +10,13 @@ pub enum Eip7702Error { #[from] Signature(alloy_primitives::SignatureError), } + +#[cfg(feature = "std")] +impl std::error::Error for Eip7702Error { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + Self::InvalidSValue(_) => None, + Self::Signature(err) => Some(err), + } + } +} From f6313812525d579e7e788cb31ec9bd1f67224198 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Tue, 22 Oct 2024 17:46:38 +0400 Subject: [PATCH 3/4] fix --- crates/eip7702/Cargo.toml | 4 ++-- crates/eip7702/src/auth_list.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/eip7702/Cargo.toml b/crates/eip7702/Cargo.toml index 190d6bc..156062e 100644 --- a/crates/eip7702/Cargo.toml +++ b/crates/eip7702/Cargo.toml @@ -32,7 +32,7 @@ arbitrary = { workspace = true, features = ["derive"], optional = true } k256 = { workspace = true, optional = true } rand = { workspace = true, optional = true } -derive_more = "1" +derive_more = { version = "1", default-features = false } [dev-dependencies] bincode = "1.3" @@ -41,7 +41,7 @@ serde_json.workspace = true [features] default = ["std"] -std = ["alloy-primitives/std", "alloy-rlp/std", "serde?/std"] +std = ["alloy-primitives/std", "alloy-rlp/std", "serde?/std", "derive_more/std"] serde = ["dep:serde", "alloy-primitives/serde"] serde-bincode-compat = ["serde_with"] arbitrary = ["std", "dep:arbitrary", "dep:rand", "alloy-primitives/arbitrary"] diff --git a/crates/eip7702/src/auth_list.rs b/crates/eip7702/src/auth_list.rs index d44f47e..58309f4 100644 --- a/crates/eip7702/src/auth_list.rs +++ b/crates/eip7702/src/auth_list.rs @@ -129,7 +129,7 @@ impl SignedAuthorization { /// not be constructed from vrs values. /// /// Note that this signature might still be invalid for recovery as it might have `s` value - /// greater than [`SECP256K1N_HALF`]. + /// greater than [secp256k1n/2](crate::constants::SECP256K1N_HALF). pub fn signature(&self) -> Result { if self.y_parity() <= 1 { Ok(Signature::new(self.r, self.s, Parity::Parity(self.y_parity() == 1))) From 45eba404db5d4f93aafe76621cca1976bc3df896 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Tue, 22 Oct 2024 17:47:55 +0400 Subject: [PATCH 4/4] rm core::error --- crates/eip7702/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/eip7702/src/error.rs b/crates/eip7702/src/error.rs index 5fca484..b63ab8a 100644 --- a/crates/eip7702/src/error.rs +++ b/crates/eip7702/src/error.rs @@ -13,7 +13,7 @@ pub enum Eip7702Error { #[cfg(feature = "std")] impl std::error::Error for Eip7702Error { - fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { Self::InvalidSValue(_) => None, Self::Signature(err) => Some(err),