Submitted by g_x_w
https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/account.cairo#L83
https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/utils/dict.cairo#L83
In CairoZero, the correct usage of dict objects created via default_dict_new must be paired with a call to default_dict_finalize to ensure the integrity and prevent malicious provers manipulation of its contents. However, this constraint is missing in the handling of transient_storage, storage and valid_jumpdests, leading to severe vulnerabilities when executing smart contracts.
According to CairoZeros documentation (link to default_dict), a proper workflow involving default_dict_new includes a finalization step using default_dict_finalize. This ensures the correct initialization of dictionary elements and prevents malicious provers from manipulating dictionary values through hints. Specifically, default_dict_finalize enforces the constraint that the initial value of the first elements prev_value in the dictionary must equal to the default_value.
However, in the case of transient_storage, storage and valid_jumpdests, this crucial constraint is missing. I will illustrate this issue using transient_storage. First, transient_storage is initialized in Account.init() as follows:
However, there is no subsequent call to default_dict_finalize(transient_storage_start, transient_storage, 0) to finalize the storage. Instead, the function default_dict_copy() is called on transient_storage multiple times during a transaction through the Account.copy() function:
This copy operation starts by calling dict_squash on the original transient_storage:
dict_squash itself does not assert the prev_value of the first element in the dictionary.
As a result, the subsequent copied transient_storages initial value is also under the malicious provers control.
If we look at the source code of default_dict_finalize, we will notice an extra constraint in the default_dict_finalize_inner function:
As shown above, the additional check besides dict_squash is:
This constraint ensures that any uninitialized read from the dictionary returns the correct default value (0 in this case). However, in default_dict_copy, this constraint is absent; meaning the prev_value for the first dictionary entry of transient_storage is not guaranteed to match the expected default value.
A malicious prover could manipulate the value read from transient_storage, storage and valid_jumpdests. Specifically, they could fabricate a proof where uninitialized keys in the dictionary return values other than the intended default (0). This could lead to unintended or unauthorized access to funds, manipulation of contract state, or other security breaches depending on the logic in the upper-level EVM contract.
Invalid Validation
ClementWalter (Kakarot) confirmed and commented:
ClementWalter (Kakarot) commented:
EV_om (Zenith) commented:
ClementWalter (Kakarot) commented:
EV_om (Zenith) commented:
3docSec (Zenith) commented:
ClementWalter (Kakarot) commented:
Kakarot mitigated:
Status: Mitigation confirmed.
