Skip to content

Commit

Permalink
fix: returndatacopy error when out of bounds (#975)
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: 0.2

## 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. -->

- Fixes returndatacopy that was able to read out of bounds
-
-

<!-- 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/975)
<!-- Reviewable:end -->
  • Loading branch information
enitrat authored Feb 13, 2024
1 parent 46d68c8 commit 37b4133
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 15 deletions.
14 changes: 0 additions & 14 deletions blockchain-tests-skip.yml
Original file line number Diff line number Diff line change
Expand Up @@ -433,20 +433,6 @@ testname:
- returndatacopy_initial_d0g0v0_Shanghai
- returndatacopy_overrun_d0g0v0_Shanghai
- subcallReturnMoreThenExpected_d0g0v0_Shanghai
- tooLongReturnDataCopy_d10g0v0_Shanghai
- tooLongReturnDataCopy_d11g0v0_Shanghai
- tooLongReturnDataCopy_d13g0v0_Shanghai
- tooLongReturnDataCopy_d16g0v0_Shanghai
- tooLongReturnDataCopy_d18g0v0_Shanghai
- tooLongReturnDataCopy_d1g0v0_Shanghai
- tooLongReturnDataCopy_d20g0v0_Shanghai
- tooLongReturnDataCopy_d21g0v0_Shanghai
- tooLongReturnDataCopy_d22g0v0_Shanghai
- tooLongReturnDataCopy_d23g0v0_Shanghai
- tooLongReturnDataCopy_d4g0v0_Shanghai
- tooLongReturnDataCopy_d6g0v0_Shanghai
- tooLongReturnDataCopy_d8g0v0_Shanghai
- tooLongReturnDataCopy_d9g0v0_Shanghai
stRevertTest:
- RevertInCallCode_d0g0v0_Shanghai
- RevertInDelegateCall_d0g0v0_Shanghai
Expand Down
31 changes: 31 additions & 0 deletions src/kakarot/errors.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,37 @@ namespace Errors {
dw 119; // w
}

func outOfBoundsRead() -> (error_len: felt, error: felt*) {
let (error) = get_label_location(out_of_bounds_read_error_message);
return (24, error);
out_of_bounds_read_error_message:
dw 'K';
dw 'a';
dw 'k';
dw 'a';
dw 'r';
dw 'o';
dw 't';
dw ':';
dw ' ';
dw 'O';
dw 'u';
dw 't';
dw 'O';
dw 'f';
dw 'B';
dw 'o';
dw 'u';
dw 'n';
dw 'd';
dw 's';
dw 'R';
dw 'e';
dw 'a';
dw 'd';
}

func unknownPrecompile(address: felt) -> (error_len: felt, error: felt*) {
alloc_locals;
let (error) = alloc();
Expand Down
15 changes: 14 additions & 1 deletion src/kakarot/instructions/environmental_information.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin
from starkware.cairo.common.cairo_keccak.keccak import cairo_keccak_bigend, finalize_keccak
from starkware.cairo.common.memset import memset
from starkware.cairo.common.math import unsigned_div_rem
from starkware.cairo.common.math_cmp import is_not_zero
from starkware.cairo.common.math_cmp import is_not_zero, is_le

from kakarot.account import Account
from kakarot.evm import EVM
from kakarot.errors import Errors
from kakarot.gas import Gas
from kakarot.memory import Memory
from kakarot.model import model
Expand Down Expand Up @@ -205,15 +206,27 @@ namespace EnvironmentalInformation {
if (opcode_number == 0x37) {
assert data_len = evm.message.calldata_len;
assert data = evm.message.calldata;
tempvar range_check_ptr = range_check_ptr;
} else {
if (opcode_number == 0x39) {
assert data_len = evm.message.bytecode_len;
assert data = evm.message.bytecode;
tempvar range_check_ptr = range_check_ptr;
} else {
tempvar is_in_bounds = is_le(offset.low + size.low, evm.return_data_len);
if (is_in_bounds == FALSE) {
let (revert_reason_len, revert_reason) = Errors.outOfBoundsRead();
let evm = EVM.stop(
evm, revert_reason_len, revert_reason, Errors.EXCEPTIONAL_HALT
);
return evm;
}
assert data_len = evm.return_data_len;
assert data = evm.return_data;
tempvar range_check_ptr = range_check_ptr;
}
}
let range_check_ptr = [ap - 1];
slice(sliced_data, data_len, data, offset.low, size.low);

Memory.store_n(size.low, sliced_data, dest_offset.low);
Expand Down

0 comments on commit 37b4133

Please sign in to comment.