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: OOB read in parse_storage_keys on malformed inputs #196

Merged
merged 1 commit into from
Dec 3, 2024
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
9 changes: 9 additions & 0 deletions cairo/src/utils/transaction.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,10 @@ namespace Transaction {

// Address
let address_item = cast(access_list.data, RLP.Item*);
with_attr error_message("Invalid address length") {
assert [range_check_ptr] = address_item.data_len - 20;
}
let range_check_ptr = range_check_ptr + 1;
let address = Helpers.bytes20_to_felt(address_item.data);

// List<StorageKeys>
Expand Down Expand Up @@ -320,6 +324,11 @@ namespace Transaction {
return ();
}

with_attr error_message("Invalid storage key length") {
assert [range_check_ptr] = keys_list.data_len - 32;
}
let range_check_ptr = range_check_ptr + 1;

let key = Helpers.bytes32_to_uint256(keys_list.data);
assert [parsed_keys] = key.low;
assert [parsed_keys + 1] = key.high;
Expand Down
28 changes: 27 additions & 1 deletion cairo/tests/src/utils/test_transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
from hypothesis import strategies as st
from rlp import encode

from tests.utils.constants import INVALID_TRANSACTIONS, TRANSACTIONS
from tests.utils.constants import (
ACCESS_LIST_TRANSACTION,
INVALID_TRANSACTIONS,
TRANSACTIONS,
)
from tests.utils.errors import cairo_error
from tests.utils.helpers import flatten_tx_access_list, rlp_encode_signed_data

Expand Down Expand Up @@ -144,6 +148,28 @@ def test_should_parse_access_list(self, cairo_run, transaction):
expected_output = flatten_tx_access_list(transaction.get("accessList", []))
assert output == expected_output

def test_should_panic_on_invalid_address_format(self, cairo_run):
rlp_structure_tx = transaction_rpc_to_rlp_structure(ACCESS_LIST_TRANSACTION)
# modify access list for addr to be 1 byte
rlp_structure_tx["accessList"] = [
(f"0x{bytes([1]).hex()}", storage_keys)
for _, storage_keys in rlp_structure_tx["accessList"]
]
encoded_access_list = encode(rlp_structure_tx.get("accessList", []))
with cairo_error("Invalid address length"):
cairo_run("test__parse_access_list", data=list(encoded_access_list))

def test_should_panic_on_invalid_storage_key_format(self, cairo_run):
rlp_structure_tx = transaction_rpc_to_rlp_structure(ACCESS_LIST_TRANSACTION)
# modify access list for storage key to be 1 byte
rlp_structure_tx["accessList"] = [
(address, (f"0x{bytes([1]).hex()}",))
for address, _ in rlp_structure_tx["accessList"]
]
encoded_access_list = encode(rlp_structure_tx.get("accessList", []))
with cairo_error("Invalid storage key length"):
cairo_run("test__parse_access_list", data=list(encoded_access_list))

class TestGetTxType:
@pytest.mark.parametrize("transaction", TRANSACTIONS)
def test_should_return_tx_type(self, cairo_run, transaction):
Expand Down
38 changes: 20 additions & 18 deletions cairo/tests/utils/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,25 @@
}
OWNER, OTHER = signers.keys()

ACCESS_LIST_TRANSACTION = {
"type": 1,
"gas": 100_000,
"gasPrice": 1_000_000_000,
"data": "0x616263646566",
"nonce": 34,
"to": "0x09616C3d61b3331fc4109a9E41a8BDB7d9776609",
"value": 0x5AF3107A4000,
"accessList": (
{
"address": "0x0000000000000000000000000000000000000001",
"storageKeys": (
"0x0100000000000000000000000000000000000000000000000000000000000000",
),
},
),
"chainId": CHAIN_ID,
}

# Taken from eth_account.account.Account.sign_transaction docstring
# https://eth-account.readthedocs.io/en/stable/eth_account.html?highlight=sign_transaction#eth_account.account.Account.sign_transaction
TRANSACTIONS = [
Expand All @@ -43,24 +62,7 @@
"chainId": CHAIN_ID,
"data": b"",
},
{
"type": 1,
"gas": 100_000,
"gasPrice": 1_000_000_000,
"data": "0x616263646566",
"nonce": 34,
"to": "0x09616C3d61b3331fc4109a9E41a8BDB7d9776609",
"value": 0x5AF3107A4000,
"accessList": (
{
"address": "0x0000000000000000000000000000000000000001",
"storageKeys": (
"0x0100000000000000000000000000000000000000000000000000000000000000",
),
},
),
"chainId": CHAIN_ID,
},
ACCESS_LIST_TRANSACTION,
# Access list with two addresses
{
"type": 1,
Expand Down
Loading