From 177f3d50d4d26cbc2fd8ed5ab8bec5f9579caf2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Walter?= Date: Thu, 5 Sep 2024 12:48:06 +0200 Subject: [PATCH 1/2] Assert on item_len before helpers --- src/utils/eth_transaction.cairo | 17 +++++++- tests/src/utils/test_eth_transaction.py | 56 +++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/utils/eth_transaction.cairo b/src/utils/eth_transaction.cairo index a272b1688..1e66c664f 100644 --- a/src/utils/eth_transaction.cairo +++ b/src/utils/eth_transaction.cairo @@ -4,7 +4,7 @@ from starkware.cairo.common.alloc import alloc from starkware.cairo.common.bool import TRUE, FALSE from starkware.cairo.common.cairo_builtins import BitwiseBuiltin, HashBuiltin from starkware.cairo.common.math_cmp import is_not_zero, is_nn -from starkware.cairo.common.math import assert_not_zero +from starkware.cairo.common.math import assert_not_zero, assert_nn from starkware.cairo.common.memcpy import memcpy from starkware.cairo.common.uint256 import Uint256 @@ -45,12 +45,16 @@ namespace EthTransaction { assert items[4].is_list = FALSE; assert items[5].is_list = FALSE; + assert_nn(31 - items[0].data_len); let nonce = Helpers.bytes_to_felt(items[0].data_len, items[0].data); + assert_nn(31 - items[1].data_len); let gas_price = Helpers.bytes_to_felt(items[1].data_len, items[1].data); + assert_nn(31 - items[2].data_len); let gas_limit = Helpers.bytes_to_felt(items[2].data_len, items[2].data); let destination = Helpers.try_parse_destination_from_bytes( items[3].data_len, items[3].data ); + assert_nn(32 - items[4].data_len); let amount = Helpers.bytes_to_uint256(items[4].data_len, items[4].data); let payload_len = items[5].data_len; let payload = items[5].data; @@ -114,13 +118,18 @@ namespace EthTransaction { assert items[6].is_list = FALSE; assert items[7].is_list = TRUE; + assert_nn(31 - items[0].data_len); let chain_id = Helpers.bytes_to_felt(items[0].data_len, items[0].data); + assert_nn(31 - items[1].data_len); let nonce = Helpers.bytes_to_felt(items[1].data_len, items[1].data); + assert_nn(31 - items[2].data_len); let gas_price = Helpers.bytes_to_felt(items[2].data_len, items[2].data); + assert_nn(31 - items[3].data_len); let gas_limit = Helpers.bytes_to_felt(items[3].data_len, items[3].data); let destination = Helpers.try_parse_destination_from_bytes( items[4].data_len, items[4].data ); + assert_nn(32 - items[5].data_len); let amount = Helpers.bytes_to_uint256(items[5].data_len, items[5].data); let payload_len = items[6].data_len; let payload = items[6].data; @@ -172,14 +181,20 @@ namespace EthTransaction { assert items[7].is_list = FALSE; assert items[8].is_list = TRUE; + assert_nn(31 - items[0].data_len); let chain_id = Helpers.bytes_to_felt(items[0].data_len, items[0].data); + assert_nn(31 - items[1].data_len); let nonce = Helpers.bytes_to_felt(items[1].data_len, items[1].data); + assert_nn(31 - items[2].data_len); let max_priority_fee_per_gas = Helpers.bytes_to_felt(items[2].data_len, items[2].data); + assert_nn(31 - items[3].data_len); let max_fee_per_gas = Helpers.bytes_to_felt(items[3].data_len, items[3].data); + assert_nn(31 - items[4].data_len); let gas_limit = Helpers.bytes_to_felt(items[4].data_len, items[4].data); let destination = Helpers.try_parse_destination_from_bytes( items[5].data_len, items[5].data ); + assert_nn(32 - items[6].data_len); let amount = Helpers.bytes_to_uint256(items[6].data_len, items[6].data); let payload_len = items[7].data_len; let payload = items[7].data; diff --git a/tests/src/utils/test_eth_transaction.py b/tests/src/utils/test_eth_transaction.py index 1e6569bf7..664d00b2b 100644 --- a/tests/src/utils/test_eth_transaction.py +++ b/tests/src/utils/test_eth_transaction.py @@ -1,5 +1,10 @@ import pytest from eth_account._utils.transaction_utils import transaction_rpc_to_rlp_structure +from eth_account._utils.validation import LEGACY_TRANSACTION_VALID_VALUES +from eth_account.typed_transactions.access_list_transaction import AccessListTransaction +from eth_account.typed_transactions.dynamic_fee_transaction import DynamicFeeTransaction +from hypothesis import given +from hypothesis import strategies as st from rlp import encode from tests.utils.constants import INVALID_TRANSACTIONS, TRANSACTIONS @@ -23,6 +28,57 @@ async def test_should_raise_with_list_items(self, cairo_run): with cairo_error(): cairo_run("test__decode", data=list(encode(list(transaction.values())))) + @given(value=st.integers(min_value=2**248)) + @pytest.mark.parametrize("transaction", TRANSACTIONS) + @pytest.mark.parametrize( + "key", + [ + "nonce", + "gasPrice", + "gas", + "value", + "chainId", + "maxFeePerGas", + "maxPriorityFeePerGas", + ], + ) + async def test_should_raise_with_params_overflow( + self, cairo_run, transaction, key, value + ): + # Not all transactions have all keys + if key not in transaction: + return + + # Value can be up to 32 bytes + if key == "value": + value *= 256 + + # Override the value + transaction = {**transaction, key: value} + + tx_type = transaction.pop("type", 0) + # Remove accessList from the transaction if it exists, not relevant for this test + if tx_type > 0: + transaction["accessList"] = [] + + # Encode the transaction + encoded_unsigned_tx = ( + b"" if tx_type == 0 else tx_type.to_bytes(1, "big") + ) + encode( + [ + transaction[key] + for key in [ + LEGACY_TRANSACTION_VALID_VALUES.keys(), + dict(AccessListTransaction.unsigned_transaction_fields).keys(), + dict(DynamicFeeTransaction.unsigned_transaction_fields).keys(), + ][tx_type] + ] + ) + + # Run the test + with cairo_error(): + cairo_run("test__decode", data=list(encoded_unsigned_tx)) + @pytest.mark.parametrize("transaction", TRANSACTIONS) async def test_should_decode_all_transactions_types( self, cairo_run, transaction From db87500055b9664d0d3cbcabc4b1d5e56e9e1dbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Walter?= Date: Thu, 5 Sep 2024 13:43:51 +0200 Subject: [PATCH 2/2] Fix kakarot tests --- tests/src/kakarot/test_kakarot.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/src/kakarot/test_kakarot.py b/tests/src/kakarot/test_kakarot.py index fe01e6aec..4dd504c23 100644 --- a/tests/src/kakarot/test_kakarot.py +++ b/tests/src/kakarot/test_kakarot.py @@ -8,7 +8,6 @@ from eth_utils.address import to_checksum_address from hypothesis import given from hypothesis.strategies import composite, integers -from starkware.cairo.lang.cairo_constants import DEFAULT_PRIME from starkware.starknet.public.abi import get_storage_var_address from web3._utils.abi import map_abi_data from web3._utils.normalizers import BASE_RETURN_NORMALIZERS @@ -453,7 +452,7 @@ def test_should_raise_invalid_nonce(self, cairo_run, tx): tx_data=tx_data, ) - @given(gas_limit=integers(min_value=2**64, max_value=DEFAULT_PRIME - 1)) + @given(gas_limit=integers(min_value=2**64, max_value=2**248 - 1)) def test_raise_gas_limit_too_high(self, cairo_run, gas_limit): tx = { "type": 2, @@ -479,7 +478,7 @@ def test_raise_gas_limit_too_high(self, cairo_run, gas_limit): tx_data=tx_data, ) - @given(maxFeePerGas=integers(min_value=2**128, max_value=DEFAULT_PRIME - 1)) + @given(maxFeePerGas=integers(min_value=2**128, max_value=2**248 - 1)) def test_raise_max_fee_per_gas_too_high(self, cairo_run, maxFeePerGas): tx = { "type": 2, @@ -539,7 +538,7 @@ def test_raise_max_fee_per_gas_too_low(self, cairo_run, tx): def max_priority_fee_too_high(draw): max_fee_per_gas = draw(integers(min_value=0, max_value=2**128 - 2)) max_priority_fee_per_gas = draw( - integers(min_value=max_fee_per_gas + 1, max_value=DEFAULT_PRIME - 1) + integers(min_value=max_fee_per_gas + 1, max_value=2**248 - 1) ) return (max_fee_per_gas, max_priority_fee_per_gas)