Skip to content

Commit

Permalink
feat: whitelist 'compute_starknet_address' from reentrancy (#1254)
Browse files Browse the repository at this point in the history
<!--- Please provide a general summary of your changes in the title
above -->

<!-- Give an estimate of the time you spent on this PR in terms of work
days.
Did you spend 0.5 days on this PR or rather 2 days?  -->

Time spent on this PR:

## Pull request type

<!-- Please try to limit your pull request to one type,
submit multiple pull requests if needed. -->

Please check the type of change your PR introduces:

- [ ] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

<!-- Please describe the current behavior that you are modifying,
or link to a relevant issue. -->

Resolves #<Issue number>

## What is the new behavior?

<!-- Please describe the behavior or changes that are being added by
this PR. -->

-
-
-

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg"
height="34" align="absmiddle"
alt="Reviewable"/>](https://reviewable.io/reviews/kkrt-labs/kakarot/1254)
<!-- Reviewable:end -->
  • Loading branch information
enitrat authored Jul 5, 2024
1 parent 59835a7 commit de5b7d8
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 14 deletions.
15 changes: 12 additions & 3 deletions src/kakarot/accounts/account_contract.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ from kakarot.accounts.library import (
Account_authorized_message_hashes,
)
from kakarot.accounts.model import CallArray
from utils.utils import Helpers
from kakarot.errors import Errors
from kakarot.interfaces.interfaces import IKakarot, IAccount
from starkware.starknet.common.syscalls import (
get_tx_info,
Expand All @@ -26,6 +28,8 @@ from starkware.starknet.common.syscalls import (
from starkware.cairo.common.math import assert_le, unsigned_div_rem
from starkware.cairo.common.alloc import alloc

const COMPUTE_STARKNET_ADDRESS_SELECTOR = 0x0ad7772990f7f5a506d84e5723efd1242e989c23f45653870d49d6d107f6e7;

// @title EVM smart contract account representation.
@constructor
func constructor{
Expand Down Expand Up @@ -317,9 +321,14 @@ func execute_starknet_call{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range
) -> (retdata_len: felt, retdata: felt*, success: felt) {
Ownable.assert_only_owner();
let (kakarot_address) = Ownable.owner();
if (kakarot_address == to) {
let (retdata) = alloc();
return (0, retdata, FALSE);
let is_compute_starknet_address = Helpers.is_zero(
COMPUTE_STARKNET_ADDRESS_SELECTOR - function_selector
);
let is_kakarot = Helpers.is_zero(kakarot_address - to);
tempvar is_forbidden = is_kakarot * (1 - is_compute_starknet_address);
if (is_forbidden != FALSE) {
let (error_len, error) = Errors.kakarotReentrancy();
return (error_len, error, FALSE);
}
let (retdata_len, retdata) = call_contract(to, function_selector, calldata_len, calldata);
return (retdata_len, retdata, TRUE);
Expand Down
26 changes: 26 additions & 0 deletions src/kakarot/errors.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -744,4 +744,30 @@ namespace Errors {
dw 'o';
dw 'r';
}

func kakarotReentrancy() -> (error_len: felt, error: felt*) {
let (error) = get_label_location(kakarot_reentrancy_error_message);
return (19, error);
kakarot_reentrancy_error_message:
dw 'K';
dw 'a';
dw 'k';
dw 'a';
dw 'r';
dw 'o';
dw 't';
dw ':';
dw ' ';
dw 'r';
dw 'e';
dw 'e';
dw 'n';
dw 't';
dw 'r';
dw 'a';
dw 'n';
dw 'c';
dw 'y';
}
}
23 changes: 13 additions & 10 deletions src/kakarot/precompiles/kakarot_precompiles.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ from starkware.cairo.common.math import assert_not_zero
from starkware.cairo.common.alloc import alloc
from starkware.starknet.common.syscalls import call_contract, library_call, get_caller_address
from starkware.starknet.common.messages import send_message_to_l1
from starkware.cairo.common.bool import FALSE
from starkware.cairo.common.bool import FALSE, TRUE

from kakarot.errors import Errors
from kakarot.interfaces.interfaces import IAccount
Expand Down Expand Up @@ -48,9 +48,7 @@ namespace KakarotPrecompiles {
let is_input_invalid = is_le(input_len, 99);
if (is_input_invalid != 0) {
let (revert_reason_len, revert_reason) = Errors.outOfBoundsRead();
return (
revert_reason_len, revert_reason, CAIRO_PRECOMPILE_GAS, Errors.EXCEPTIONAL_HALT
);
return (revert_reason_len, revert_reason, CAIRO_PRECOMPILE_GAS, TRUE);
}

// Input is formatted as:
Expand Down Expand Up @@ -100,10 +98,15 @@ namespace KakarotPrecompiles {
let (retdata_len, retdata, success) = IAccount.execute_starknet_call(
caller_starknet_address, to_starknet_address, starknet_selector, data_len, data
);
if (success == FALSE) {
// skip formatting to bytes32 array and return revert reason directly
return (retdata_len, retdata, CAIRO_PRECOMPILE_GAS, TRUE);
}

let (output) = alloc();
let output_len = retdata_len * 32;
Helpers.felt_array_to_bytes32_array(retdata_len, retdata, output);
return (output_len, output, CAIRO_PRECOMPILE_GAS, 1 - success);
return (output_len, output, CAIRO_PRECOMPILE_GAS, FALSE);
}

if (selector == LIBRARY_CALL_SOLIDITY_SELECTOR) {
Expand All @@ -113,11 +116,11 @@ namespace KakarotPrecompiles {
let (output) = alloc();
let output_len = retdata_len * 32;
Helpers.felt_array_to_bytes32_array(retdata_len, retdata, output);
return (output_len, output, CAIRO_PRECOMPILE_GAS, 0);
return (output_len, output, CAIRO_PRECOMPILE_GAS, FALSE);
}

let (revert_reason_len, revert_reason) = Errors.invalidCairoSelector();
return (revert_reason_len, revert_reason, CAIRO_PRECOMPILE_GAS, Errors.EXCEPTIONAL_HALT);
return (revert_reason_len, revert_reason, CAIRO_PRECOMPILE_GAS, TRUE);
}

// @notice Sends a message to a message to L1.
Expand All @@ -138,7 +141,7 @@ namespace KakarotPrecompiles {
let is_input_invalid = is_le(input_len, 95);
if (is_input_invalid != 0) {
let (revert_reason_len, revert_reason) = Errors.outOfBoundsRead();
return (revert_reason_len, revert_reason, CAIRO_MESSAGE_GAS, Errors.EXCEPTIONAL_HALT);
return (revert_reason_len, revert_reason, CAIRO_MESSAGE_GAS, TRUE);
}

// Input is formatted as:
Expand All @@ -151,7 +154,7 @@ namespace KakarotPrecompiles {
let data_fits_in_input = is_le(data_bytes_len, input_len - 3 * 32);
if (data_fits_in_input == 0) {
let (revert_reason_len, revert_reason) = Errors.outOfBoundsRead();
return (revert_reason_len, revert_reason, CAIRO_MESSAGE_GAS, Errors.EXCEPTIONAL_HALT);
return (revert_reason_len, revert_reason, CAIRO_MESSAGE_GAS, TRUE);
}
let data_ptr = input + 3 * 32;

Expand All @@ -160,6 +163,6 @@ namespace KakarotPrecompiles {

send_message_to_l1(target_address, data_bytes_len, data_ptr);
let (output) = alloc();
return (0, output, CAIRO_MESSAGE_GAS, 0);
return (0, output, CAIRO_MESSAGE_GAS, FALSE);
}
}
2 changes: 1 addition & 1 deletion src/kakarot/precompiles/precompiles.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ namespace Precompiles {
bitwise_ptr: BitwiseBuiltin*,
}() -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) {
let (revert_reason_len, revert_reason) = Errors.unauthorizedPrecompile();
return (revert_reason_len, revert_reason, 0, Errors.EXCEPTIONAL_HALT);
return (revert_reason_len, revert_reason, 0, Errors.REVERT);
}

// @notice A placeholder for precompile that don't exist.
Expand Down

0 comments on commit de5b7d8

Please sign in to comment.