feat: check signature s during recovery (#14)

* feat: check signature s during recovery

* error impl

* fix

* rm core::error

crates/eip7702/Cargo.toml

@@ -32,14 +32,16 @@ arbitrary = { workspace = true, features = ["derive"], optional = true }
 k256 = { workspace = true, optional = true }
 rand = { workspace = true, optional = true }
 
+derive_more = { version = "1", default-features = false }
+
 [dev-dependencies]
 bincode = "1.3"
 rand = "0.8"
 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"]

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/2](crate::constants::SECP256K1N_HALF).
     pub fn signature(&self) -> Result<Signature, SignatureError> {
         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<Address, alloy_primitives::SignatureError> {
-        self.signature()?.recover_address_from_prehash(&self.inner.signature_hash())
+    pub fn recover_authority(&self) -> Result<Address, crate::error::Eip7702Error> {
+        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

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);

crates/eip7702/src/error.rs

@@ -0,0 +1,22 @@
+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),
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for Eip7702Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match self {
+            Self::InvalidSValue(_) => None,
+            Self::Signature(err) => Some(err),
+        }
+    }
+}

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