[KGA-49]: fix OOB read in parse_storage_keys on malformed inputs (#1627)

Fixes an
[issue](code-423n4/2024-09-kakarot-findings#81)
where parsing malformed access lists could have undefined behavior due
to OOB reads. This had no impact on the protocol whatsoever and could
only discard the benefits from eip2930

cairo_zero/tests/src/utils/test_eth_transaction.py

@@ -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
 
@@ -157,6 +161,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):

cairo_zero/utils/eth_transaction.cairo

@@ -290,6 +290,10 @@ namespace EthTransaction {
 
         // 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>
@@ -323,6 +327,11 @@ namespace EthTransaction {
             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;

tests/utils/constants.py

@@ -42,6 +42,25 @@
 BLOCK_NUMBER = 0x42
 BLOCK_TIMESTAMP = int(time())
 
+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 = [
@@ -54,24 +73,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,