diff --git a/cairo_zero/kakarot/accounts/library.cairo b/cairo_zero/kakarot/accounts/library.cairo index 68ca07bb3..aaca23408 100644 --- a/cairo_zero/kakarot/accounts/library.cairo +++ b/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,16 @@ namespace AccountContract { } let range_check_ptr = [ap - 1]; + // Signature validation + // `verify_eth_signature_uint256` verifies that r and s are 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 diff --git a/cairo_zero/tests/src/kakarot/accounts/test_account_contract.py b/cairo_zero/tests/src/kakarot/accounts/test_account_contract.py index ae9e37008..024fd732c 100644 --- a/cairo_zero/tests/src/kakarot/accounts/test_account_contract.py +++ b/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,34 @@ def test_should_raise_with_wrong_signature(self, cairo_run): chain_id=CHAIN_ID, ) + @given(s_value=integers(min_value=SECP256K1N // 2 + 1, max_value=SECP256K1N)) + def test_should_raise_with_high_s_values(self, cairo_run, s_value): + """Test that signatures with s values > secp256k1n/2 are rejected (EIP-2).""" + 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) + + # Override the s value with our test value while keeping r and v + signature = [ + *int_to_uint256(signed.r), + *int_to_uint256(s_value), + signed.v, + ] + 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