Add additional verification for v & s value in evm_signature_verify

massa-execution-worker/src/interface_impl.rs

@@ -885,7 +885,7 @@ impl Interface for InterfaceImpl {
         // parse the public key
         let public_key = libsecp256k1::PublicKey::parse_slice(
             public_key_,
-            Some(libsecp256k1::PublicKeyFormat::Raw),
+            None,
         )?;
 
         // build the message
@@ -899,9 +899,33 @@ impl Interface for InterfaceImpl {
         // r is the R.x value of the signature's R point (32 bytes)
         // s is the signature proof for R.x (32 bytes)
         // v is a recovery parameter used to ease the signature verification (1 byte)
-        // we ignore the recovery parameter here
         // see test_evm_verify for an example of its usage
-        let signature = libsecp256k1::Signature::parse_standard_slice(&signature_[..64])?;
+        let recovery_id: u8 = libsecp256k1::RecoveryId::parse_rpc(signature_[64])?
+            .into();
+        // Note: parse_rpc returns p - 27 and allow for 27, 28, 29, 30
+        //       restrict to only 27 & 28 (=> 0 & 1)
+        if recovery_id != 0 && recovery_id != 1 {
+            // Note:
+            // The v value in an EVM signature serves as a recovery ID,
+            // aiding in the recovery of the public key from the signature.
+            // Typically, v should be either 27 or 28
+            // (or sometimes 0 or 1, depending on the implementation).
+            // Ensuring that v is within the expected range is crucial
+            // for correctly recovering the public key.
+            // the Ethereum yellow paper specifies only 27 and 28, requiring additional checks.
+            return Err(anyhow!("invalid recovery id value (v = {recovery_id}) in evm_signature_verify"));
+        }
+
+        let mut signature = libsecp256k1::Signature::parse_standard_slice(&signature_[..64])?;
+
+        // Note:
+        // The s value in an EVM signature should be in the lower half of the elliptic curve
+        // in order to prevent malleability attacks.
+        // If s is in the high-order range, it can be converted to its low-order equivalent,
+        // which should be enforced during signature verification.
+        if signature.s.is_high() {
+            signature.s = -signature.s;
+        }
 
         // verify the signature
         Ok(libsecp256k1::verify(&message, &signature, &public_key))

massa-execution-worker/src/tests/interface.rs

@@ -17,3 +17,29 @@ fn test_hash_sha256() {
         &hex!("3fc9b689459d738f8c88a3a48aa9e33542016b7a4052e001aaa536fca74813cb")[..];
     assert_eq!(actual_hash, expected_hash);
 }
+
+#[test]
+fn test_evm_signature_verify() {
+
+    let interface = InterfaceImpl::new_default(
+        Address::from_str("AU12cMW9zRKFDS43Z2W88VCmdQFxmHjAo54XvuVV34UzJeXRLXW9M").unwrap(),
+        None,
+    );
+
+    let _address = hex!("807a7bb5193edf9898b9092c1597bb966fe52514");
+    let message_ = b"test";
+    let mut signature_ = hex!("d0d05c35080635b5e865006c6c4f5b5d457ec342564d8fc67ce40edc264ccdab3f2f366b5bd1e38582538fed7fa6282148e86af97970a10cb3302896f5d68ef51b");
+    let private_key_ = hex!("ed6602758bdd68dc9df67a6936ed69807a74b8cc89bdc18f3939149d02db17f3");
+
+    // build original public key
+    let private_key = libsecp256k1::SecretKey::parse_slice(&private_key_).unwrap();
+    let public_key = libsecp256k1::PublicKey::from_secret_key(&private_key);
+
+    let result = interface.evm_signature_verify(message_, &signature_, &public_key.serialize());
+    assert!(result.is_ok());
+
+    // Invalid v
+    signature_[64] ^= 1;
+    let result = interface.evm_signature_verify(message_, &signature_, &public_key.serialize());
+    assert!(result.is_err());
+}