fix: return error if signature len does not eq 65 (#717)

* fix: return error if signature len does not eq `65` * fix: avoid magic numbers * fix: tests * fix: return error on size check in compile time * chore: remove outdated code * chore: update tests * fix: update test outputs * Update engine-precompiles/src/secp256k1.rs Co-authored-by: Oleksandr Anyshchenko <[email protected]> --------- Co-authored-by: Oleksandr Anyshchenko <[email protected]>
aurora-is-near · Apr 5, 2023 · 71a4799 · 71a4799
1 parent 6b70e34
commit 71a4799
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 5 deletions.
diff --git a/engine-precompiles/src/secp256k1.rs b/engine-precompiles/src/secp256k1.rs
@@ -11,16 +11,18 @@ mod costs {
 
 mod consts {
     pub(super) const INPUT_LEN: usize = 128;
+    pub(super) const SIGNATURE_LENGTH: usize = 65;
 }
 
 /// See: `https://ethereum.github.io/yellowpaper/paper.pdf`
 /// See: `https://docs.soliditylang.org/en/develop/units-and-global-variables.html#mathematical-and-cryptographic-functions`
 /// See: `https://etherscan.io/address/0000000000000000000000000000000000000001`
 // Quite a few library methods rely on this and that should be changed. This
 // should only be for precompiles.
-pub fn ecrecover(hash: H256, signature: &[u8]) -> Result<Address, ExitError> {
-    assert_eq!(signature.len(), 65);
-
+pub fn ecrecover(
+    hash: H256,
+    signature: &[u8; consts::SIGNATURE_LENGTH],
+) -> Result<Address, ExitError> {
     #[cfg(feature = "contract")]
     return sdk::ecrecover(hash, signature).map_err(|e| ExitError::Other(Borrowed(e.as_str())));
 
@@ -87,7 +89,7 @@ impl Precompile for ECRecover {
         let mut v = [0; 32];
         v.copy_from_slice(&input[32..64]);
 
-        let mut signature = [0; 65]; // signature is (r, s, v), typed (uint256, uint256, uint8)
+        let mut signature = [0; consts::SIGNATURE_LENGTH]; // signature is (r, s, v), typed (uint256, uint256, uint8)
         signature[0..32].copy_from_slice(&input[64..96]); // r
         signature[32..64].copy_from_slice(&input[96..128]); // s
 
@@ -118,7 +120,7 @@ mod tests {
     use crate::utils::new_context;
 
     fn ecverify(hash: H256, signature: &[u8], signer: Address) -> bool {
-        matches!(ecrecover(hash, signature), Ok(s) if s == signer)
+        matches!(ecrecover(hash, signature[0..super::consts::SIGNATURE_LENGTH].try_into().unwrap()), Ok(s) if s == signer)
     }
 
     #[test]
@@ -247,4 +249,17 @@ mod tests {
             .output;
         assert_eq!(res, expected);
     }
+
+    #[test]
+    fn test_extra_input_length() {
+        let input = hex::decode("18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c000000000000000000000000000000000000000000000000000000000000001c73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75feeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549aabbccddeeff").unwrap();
+        let expected =
+            hex::decode("000000000000000000000000a94f5374fce5edbc8e2a8697c15331677e6ebf0b")
+                .unwrap();
+        let res = ECRecover
+            .run(&input, Some(EthGas::new(3_000)), &new_context(), false)
+            .unwrap()
+            .output;
+        assert_eq!(res, expected);
+    }
 }
diff --git a/engine-tests/src/tests/ecrecover.rs b/engine-tests/src/tests/ecrecover.rs
@@ -17,13 +17,15 @@ fn test_ecrecover_geth() {
         "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c100000000000000000000000000000000000000000000000000000000000001c73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75feeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549",
         "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c000000000000000000000000000000000000001000000000000000000000001c73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75feeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549",
         "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c000000000000000000000000000000000000001000000000000000000000011c73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75feeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549",
+        "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c000000000000000000000000000000000000000000000000000000000000001c73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75feeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549aabbccddeeff"
     ];
     let outputs = [
         Vec::new(),
         hex::decode("000000000000000000000000a94f5374fce5edbc8e2a8697c15331677e6ebf0b").unwrap(),
         Vec::new(),
         Vec::new(),
         Vec::new(),
+        hex::decode("000000000000000000000000a94f5374fce5edbc8e2a8697c15331677e6ebf0b").unwrap(),
     ];
 
     for (input, output) in inputs.iter().zip(outputs.iter()) {