diff --git a/massa-execution-worker/src/interface_impl.rs b/massa-execution-worker/src/interface_impl.rs index 1ccd389f3d..65ae41b08a 100644 --- a/massa-execution-worker/src/interface_impl.rs +++ b/massa-execution-worker/src/interface_impl.rs @@ -883,10 +883,7 @@ impl Interface for InterfaceImpl { } // parse the public key - let public_key = libsecp256k1::PublicKey::parse_slice( - public_key_, - Some(libsecp256k1::PublicKeyFormat::Raw), - )?; + let public_key = libsecp256k1::PublicKey::parse_slice(public_key_, None)?; // build the message let prefix = format!("\x19Ethereum Signed Message:\n{}", message_.len()); @@ -899,10 +896,37 @@ 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 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 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() { + return Err(anyhow!( + "High-Order s Value are prohibited in evm_get_pubkey_from_signature" + )); + } + // verify the signature Ok(libsecp256k1::verify(&message, &signature, &public_key)) } @@ -941,16 +965,33 @@ impl Interface for InterfaceImpl { } // parse the message - let message = libsecp256k1::Message::parse_slice(hash_).unwrap(); + let message = libsecp256k1::Message::parse_slice(hash_)?; // parse the signature as being (r, s, v) use only r and s - let signature = libsecp256k1::Signature::parse_standard_slice(&signature_[..64]).unwrap(); + let signature = libsecp256k1::Signature::parse_standard_slice(&signature_[..64])?; + + // Note: + // See evm_signature_verify explanation + if signature.s.is_high() { + return Err(anyhow!( + "High-Order s Value are prohibited in evm_get_pubkey_from_signature" + )); + } // parse v as a recovery id - let recovery_id = libsecp256k1::RecoveryId::parse_rpc(signature_[64]).unwrap(); + let recovery_id = libsecp256k1::RecoveryId::parse_rpc(signature_[64])?; + + let recovery_id_: u8 = recovery_id.into(); + if recovery_id_ != 0 && recovery_id_ != 1 { + // Note: + // See evm_signature_verify explanation + return Err(anyhow!( + "invalid recovery id value (v = {recovery_id_}) in evm_get_pubkey_from_signature" + )); + } // recover the public key - let recovered = libsecp256k1::recover(&message, &signature, &recovery_id).unwrap(); + let recovered = libsecp256k1::recover(&message, &signature, &recovery_id)?; // return its serialized value Ok(recovered.serialize().to_vec()) diff --git a/massa-execution-worker/src/tests/interface.rs b/massa-execution-worker/src/tests/interface.rs index 506ab937f7..6ada4d6a13 100644 --- a/massa-execution-worker/src/tests/interface.rs +++ b/massa-execution-worker/src/tests/interface.rs @@ -1,11 +1,15 @@ // Copyright (c) 2022 MASSA LABS +use std::str::FromStr; + use hex_literal::hex; +use sha2::Digest; + use massa_models::address::Address; use massa_sc_runtime::Interface; -use std::str::FromStr; use crate::interface_impl::InterfaceImpl; + #[test] fn test_hash_sha256() { let interface = InterfaceImpl::new_default( @@ -17,3 +21,76 @@ 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 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 + { + let mut signature_2_ = signature_; + signature_2_[64] ^= 1; + let result = + interface.evm_signature_verify(message_, &signature_2_, &public_key.serialize()); + assert!(result.is_err()); + } +} + +#[test] +fn test_evm_get_pubkey_from_signature() { + let interface = InterfaceImpl::new_default( + Address::from_str("AU12cMW9zRKFDS43Z2W88VCmdQFxmHjAo54XvuVV34UzJeXRLXW9M").unwrap(), + None, + ); + + // let _address = hex!("807a7bb5193edf9898b9092c1597bb966fe52514"); + let message_ = b"test"; + let 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); + + // build the hash + let prefix = format!("\x19Ethereum Signed Message:\n{}", message_.len()); + let to_hash = [prefix.as_bytes(), message_].concat(); + let full_hash = sha3::Keccak256::digest(to_hash); + + let result = interface.evm_get_pubkey_from_signature(&full_hash, &signature_); + assert!(result.is_ok()); + assert_eq!(public_key.serialize(), result.unwrap().as_ref()); + + // Invalid s + { + let mut signature_2 = + libsecp256k1::Signature::parse_standard_slice(&signature_[..64]).unwrap(); + signature_2.s = -signature_2.s; + assert!(signature_2.s.is_high()); + let result = interface.evm_get_pubkey_from_signature(&full_hash, &signature_2.serialize()); + assert!(result.is_err()); + } + + // Invalid v + { + let mut signature_2_ = signature_; + signature_2_[64] ^= 1; + let result = interface.evm_get_pubkey_from_signature(&full_hash, &signature_2_); + assert!(result.is_err()); + } +}