[KGA-109] [KGA-137] fix: validate s <= N//2 when recovering tx sender

cairo_zero/kakarot/accounts/library.cairo

@@ -7,7 +7,7 @@ from starkware.cairo.common.dict_access import DictAccess
 from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin
 from starkware.cairo.common.math import split_int
 from starkware.cairo.common.memcpy import memcpy
-from starkware.cairo.common.uint256 import Uint256
+from starkware.cairo.common.uint256 import Uint256, uint256_lt
 from starkware.cairo.common.math_cmp import is_nn, is_le_felt
 from starkware.starknet.common.syscalls import (
     StorageRead,
@@ -72,6 +72,9 @@ func transaction_executed(response_len: felt, response: felt*, success: felt, ga
 
 const BYTES_PER_FELT = 31;
 
+const SECP256K1N_DIV_2_LOW = 0x5d576e7357a4501ddfe92f46681b20a0;
+const SECP256K1N_DIV_2_HIGH = 0x7fffffffffffffffffffffffffffffff;
+
 // @title Account main library file.
 // @notice This file contains the EVM account representation logic.
 // @dev: Both EOAs and Contract Accounts are represented by this contract. Owner is expected to be Kakarot.
@@ -182,6 +185,17 @@ namespace AccountContract {
         }
         let range_check_ptr = [ap - 1];
 
+        // Signature validation
+        // `verify_eth_signature_uint256` verifies that r,s in the range [1, N[
+        // TX validation imposes s to be the range [1, N//2], see EIP-2
+
+        let (is_invalid_upper_s) = uint256_lt(
+            Uint256(SECP256K1N_DIV_2_LOW, SECP256K1N_DIV_2_HIGH), s
+        );
+        with_attr error_message("Invalid s value") {
+            assert is_invalid_upper_s = FALSE;
+        }
+
         let (local words: felt*) = alloc();
         let (words_len, last_word, last_word_num_bytes) = bytes_to_bytes8_little_endian(
             words, tx_data_len, tx_data

cairo_zero/tests/src/kakarot/accounts/test_account_contract.py

@@ -6,6 +6,7 @@
 import rlp
 from eth_account.account import Account
 from eth_utils import keccak
+from ethereum.crypto.elliptic_curve import SECP256K1N
 from hypothesis import given, settings
 from hypothesis.strategies import binary, composite, integers, lists, permutations
 from starkware.cairo.lang.cairo_constants import DEFAULT_PRIME
@@ -306,6 +307,37 @@ def test_should_raise_with_wrong_signature(self, cairo_run):
                     chain_id=CHAIN_ID,
                 )
 
+        def test_should_raise_with_malleable_signature_eip_2(self, cairo_run):
+            # EIP-2: one can take any transaction, flip the s value from s to secp256k1n -
+            # s, flip the v value (27 -> 28, 28 -> 27), and the resulting signature would still be
+            # valid.
+            transaction = TRANSACTIONS[0]
+            private_key = generate_random_private_key()
+            address = int(private_key.public_key.to_checksum_address(), 16)
+            signed = Account.sign_transaction(transaction, private_key)
+
+            # Modify s and flip v to generate a malleable signature
+            s_modified = SECP256K1N - signed.s
+            y_parity = signed.v - 2 * CHAIN_ID - 35
+            v_modified = signed.v + 1 if y_parity == 0 else signed.v - 1
+            signature = [
+                *int_to_uint256(signed.r),
+                *int_to_uint256(s_modified),
+                v_modified,
+            ]
+            tx_data = list(rlp_encode_signed_data(transaction))
+            with (
+                cairo_error(message="Invalid s value"),
+                SyscallHandler.patch("Account_evm_address", address),
+                SyscallHandler.patch("Account_nonce", transaction.get("nonce", 0)),
+            ):
+                cairo_run(
+                    "test__execute_from_outside",
+                    tx_data=tx_data,
+                    signature=signature,
+                    chain_id=CHAIN_ID,
+                )
+
         @composite
         def draw_signature_not_in_range(draw):
             # create signature with 4 elements < 2 ** 128 and one > 2 ** 128