Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix possible overflow of tx params #1393

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/utils/eth_transaction.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 3 additions & 4 deletions tests/src/kakarot/test_kakarot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)

Expand Down
56 changes: 56 additions & 0 deletions tests/src/utils/test_eth_transaction.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
Loading