Kakarot precompiles can be called in a staticcall context, allowing writes in view functions

[c4-bot-9]

Lines of code

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/precompiles/precompiles.cairo#L140

Vulnerability details

Kakarot precompiles (cairo_precompile and cairo_message) allow calling external Starknet functionality from an EVM context.

Among the checks that Kakarot makes to secure these Starknet calls, there is no check for the context not to be read_only:

File: precompiles.cairo
134:         kakarot_precompile:
135:         let is_whitelisted = KakarotPrecompiles.is_caller_whitelisted(caller_code_address);
136:         tempvar is_not_authorized = 1 - is_whitelisted;
137:         tempvar syscall_ptr = syscall_ptr;
138:         tempvar pedersen_ptr = pedersen_ptr;
139:         tempvar range_check_ptr = range_check_ptr;
140:         jmp unauthorized_call if is_not_authorized != 0;

A check on read_only would make sense because when the execution context leaves Kakarot and continues in Starknet contracts, the read_only information is not propagated to the Cairo execution which can make state-changing operations like ERC-20 token transfers.

With the contracts currently planned to be whitelisted, DualVmToken and L2KakarotMessaging, only the latter can be used for this purpose because the events emitted by DualVmToken revert in a static context. It is therefore possible to send messages to L1 from view functions and calls with a staticcall parent. For this reason, we recommended a Medium severity.

Proof of Concept

• Have a contract with a view function, which staticcalls a contract whitelisted for Kakarot precompiles which can be L2KakarotMessaging
• Have another contract call this view function (solc would make a staticcall here too)
• Cairo contracts can be called, and their state can be changed; no EVM revert is triggered

Recommended Mitigation Steps

Within the current scope, only DualVmToken and L2KakarotMessaging are to be whitelisted for cairo calls.

DualVmToken is safe from this because all its "write" operations (approve, transfer, transferFrom) have emit statements that will trigger a revert in a read-only context.

Similarly, it is recommended that L2KakarotMessaging is updated with the emission of an event in its sendMessageToL1 function.

In general, all other contracts to be whitelisted for cairo calls should adopt this or similar precautions.

Assessed type

Other

[ClementWalter]

Severity: Informative

Comment: The idea of this finding is that contracts using the cairo precompile should enforce their view declaration (which can be bypassed by a low level call) by emitting events, so that it's actually not possible to make "starknet write" operations in staticcall. This makes sense and could be added to the doc, but is not a vulnerability.

[c4-judge]

dmvt changed the severity to QA (Quality Assurance)