Skip to content

Commit

Permalink
fix: overflow check memory operations charge gas (kkrt-labs#1381)
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:

- [x] 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 kkrt-labs#1372

## What is the new behavior?

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

Overflow checks for memeory operation before charge gas

<!-- 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/1381)
<!-- Reviewable:end -->

---------

Co-authored-by: Clément Walter <clement0walter@gmail.com>
  • Loading branch information
obatirou and ClementWalter committed Sep 4, 2024
1 parent 6ebe45c commit 791f1a4
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 25 deletions.
25 changes: 9 additions & 16 deletions src/kakarot/instructions/environmental_information.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -176,19 +176,16 @@ namespace EnvironmentalInformation {
);

// Any size upper than 2**128 will cause an OOG error, considering the maximum gas for a transaction.
// here with size.low = 2**128 - 1, copy_gas_cost is 0x18000000000000000000000000000000, ie is between 2**124 and 2**125
let upper_bytes_bound = size.low + 31;
let (words, _) = unsigned_div_rem(upper_bytes_bound, 32);
let copy_gas_cost_low = words * Gas.COPY;
tempvar copy_gas_cost_high = is_not_zero(size.high) * 2 ** 128;
let copy_gas_cost = words * Gas.COPY;

// static cost handled in jump table
let evm = EVM.charge_gas(
evm, memory_expansion.cost + copy_gas_cost_low + copy_gas_cost_high
);
let evm = EVM.charge_gas(evm, memory_expansion.cost + copy_gas_cost);
if (evm.reverted != FALSE) {
return evm;
}

// OPERATION
tempvar memory = new model.Memory(
word_dict_start=memory.word_dict_start,
Expand Down Expand Up @@ -257,15 +254,13 @@ namespace EnvironmentalInformation {
);

// Any size upper than 2**128 will cause an OOG error, considering the maximum gas for a transaction.
// here with size.low = 2**128 - 1, copy_gas_cost is 0x18000000000000000000000000000000, ie is between 2**124 and 2**125
let upper_bytes_bound = size.low + 31;
let (words, _) = unsigned_div_rem(upper_bytes_bound, 32);
let copy_gas_cost_low = words * Gas.COPY;
tempvar copy_gas_cost_high = is_not_zero(size.high) * 2 ** 128;
let copy_gas_cost = words * Gas.COPY;

// static cost handled in jump table
let evm = EVM.charge_gas(
evm, memory_expansion.cost + copy_gas_cost_low + copy_gas_cost_high
);
let evm = EVM.charge_gas(evm, memory_expansion.cost + copy_gas_cost);
if (evm.reverted != FALSE) {
return evm;
}
Expand Down Expand Up @@ -394,18 +389,16 @@ namespace EnvironmentalInformation {
Gas.COLD_ACCOUNT_ACCESS;

// Any size upper than 2**128 will cause an OOG error, considering the maximum gas for a transaction.
// here with size.low = 2**128 - 1, copy_gas_cost is 0x18000000000000000000000000000000, ie is between 2**124 and 2**125
let upper_bytes_bound = size.low + 31;
let (words, _) = unsigned_div_rem(upper_bytes_bound, 32);
let copy_gas_cost_low = words * Gas.COPY;
tempvar copy_gas_cost_high = is_not_zero(size.high) * 2 ** 128;
let copy_gas_cost = words * Gas.COPY;

let memory_expansion = Gas.memory_expansion_cost_saturated(
memory.words_len, dest_offset, size
);

let evm = EVM.charge_gas(
evm, access_gas_cost + copy_gas_cost_low + copy_gas_cost_high + memory_expansion.cost
);
let evm = EVM.charge_gas(evm, access_gas_cost + copy_gas_cost + memory_expansion.cost);
if (evm.reverted != FALSE) {
return evm;
}
Expand Down
6 changes: 2 additions & 4 deletions src/kakarot/instructions/memory_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,12 @@ namespace MemoryOperations {
);

// Any size upper than 2**128 will cause an OOG error, considering the maximum gas for a transaction.
// here with size.low = 2**128 - 1, copy_gas_cost is 0x18000000000000000000000000000000, ie is between 2**124 and 2**125
let upper_bytes_bound = size.low + 31;
let (words, _) = unsigned_div_rem(upper_bytes_bound, 32);
let copy_gas_cost_low = words * Gas.COPY;
tempvar copy_gas_cost_high = is_not_zero(size.high) * 2 ** 128;

let evm = EVM.charge_gas(
evm, memory_expansion.cost + copy_gas_cost_low + copy_gas_cost_high
);
let evm = EVM.charge_gas(evm, memory_expansion.cost + copy_gas_cost_low);
if (evm.reverted != FALSE) {
return evm;
}
Expand Down
23 changes: 19 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import pytest
from dotenv import load_dotenv
from hypothesis import Verbosity, settings
from hypothesis import Phase, Verbosity, settings
from starkware.cairo.lang.instances import LAYOUTS

load_dotenv()
Expand Down Expand Up @@ -59,8 +59,23 @@ def seed(request):

pytest_plugins = ["tests.fixtures.starknet"]

settings.register_profile("ci", deadline=None, max_examples=1000)
settings.register_profile("dev", deadline=None, max_examples=10)
settings.register_profile("debug", max_examples=10, verbosity=Verbosity.verbose)
settings.register_profile(
"ci",
deadline=None,
max_examples=1000,
phases=[Phase.explicit, Phase.reuse, Phase.generate, Phase.target],
)
settings.register_profile(
"dev",
deadline=None,
max_examples=10,
phases=[Phase.explicit, Phase.reuse, Phase.generate, Phase.target],
)
settings.register_profile(
"debug",
max_examples=10,
verbosity=Verbosity.verbose,
phases=[Phase.explicit, Phase.reuse, Phase.generate, Phase.target],
)
settings.load_profile(os.getenv("HYPOTHESIS_PROFILE", "default"))
logger.info(f"Using Hypothesis profile: {os.getenv('HYPOTHESIS_PROFILE', 'default')}")
37 changes: 36 additions & 1 deletion tests/src/kakarot/instructions/test_memory_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ func test__exec_mload_should_load_a_value_from_memory_with_offset_larger_than_ms
let (bytecode) = alloc();
let evm = TestHelpers.init_evm_with_bytecode(0, bytecode);
let test_offset = 684;
// Given
let stack = Stack.init();
let state = State.init();
let memory = Memory.init();
Expand All @@ -169,3 +168,39 @@ func test__exec_mload_should_load_a_value_from_memory_with_offset_larger_than_ms
assert memory.words_len = 23;
return ();
}

func test__exec_mcopy{
syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
}() -> (model.EVM*, model.Memory*) {
alloc_locals;
let (memory_init_state) = alloc();
local memory_init_state_len: felt;
let (size_mcopy_ptr) = alloc();
let (src_offset_mcopy_ptr) = alloc();
let (dst_offset_mcopy_ptr) = alloc();

%{
ids.memory_init_state_len = len(program_input["memory_init_state"])
segments.write_arg(ids.memory_init_state, program_input["memory_init_state"])
segments.write_arg(ids.size_mcopy_ptr, program_input["size_mcopy"])
segments.write_arg(ids.src_offset_mcopy_ptr, program_input["src_offset_mcopy"])
segments.write_arg(ids.dst_offset_mcopy_ptr, program_input["dst_offset_mcopy"])
%}

let size_mcopy = cast(size_mcopy_ptr, Uint256*);
let src_offset_mcopy = cast(src_offset_mcopy_ptr, Uint256*);
let dst_offset_mcopy = cast(dst_offset_mcopy_ptr, Uint256*);

let evm = TestHelpers.init_evm();
let stack = Stack.init();
let state = State.init();
let memory = TestHelpers.init_memory_with_values(memory_init_state_len, memory_init_state);

with stack, memory, state {
Stack.push(size_mcopy);
Stack.push(src_offset_mcopy);
Stack.push(dst_offset_mcopy);
let evm = MemoryOperations.exec_mcopy(evm);
}
return (evm, memory);
}
78 changes: 78 additions & 0 deletions tests/src/kakarot/instructions/test_memory_operations.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import pytest
from hypothesis import example, given
from hypothesis.strategies import binary, integers
from starkware.cairo.lang.cairo_constants import DEFAULT_PRIME

from kakarot_scripts.utils.uint256 import int_to_uint256


class TestMemoryOperations:
Expand Down Expand Up @@ -28,3 +33,76 @@ def test_should_load_a_value_from_memory_with_offset_larger_than_msize(
cairo_run(
"test__exec_mload_should_load_a_value_from_memory_with_offset_larger_than_msize"
)

class TestMcopy:

@given(
memory_init_state=binary(min_size=1, max_size=100),
size_mcopy=integers(min_value=1, max_value=100),
src_offset_mcopy=integers(min_value=0, max_value=100),
dst_offset_mcopy=integers(min_value=0, max_value=100),
)
def test_should_copy_a_value_from_memory(
self,
cairo_run,
memory_init_state,
size_mcopy,
src_offset_mcopy,
dst_offset_mcopy,
):
(evm, memory) = cairo_run(
"test__exec_mcopy",
memory_init_state=memory_init_state,
size_mcopy=int_to_uint256(size_mcopy),
src_offset_mcopy=int_to_uint256(src_offset_mcopy),
dst_offset_mcopy=int_to_uint256(dst_offset_mcopy),
)
expected_memory_state = list(memory_init_state) + [0] * (
max(src_offset_mcopy, dst_offset_mcopy)
+ size_mcopy
- len(memory_init_state)
)
expected_memory_state[dst_offset_mcopy : dst_offset_mcopy + size_mcopy] = (
expected_memory_state[src_offset_mcopy : src_offset_mcopy + size_mcopy]
)
words_len = (len(expected_memory_state) + 31) // 32
expected_memory_state = expected_memory_state + [0] * (
words_len * 32 - len(expected_memory_state)
)
assert bytes.fromhex(memory) == bytes(expected_memory_state)

@given(
memory_init_state=binary(min_size=1, max_size=100),
size_mcopy=integers(min_value=2**128, max_value=DEFAULT_PRIME - 1),
src_offset_mcopy=integers(min_value=0, max_value=100),
dst_offset_mcopy=integers(min_value=0, max_value=100),
)
@example(
memory_init_state=b"a" * 100,
size_mcopy=2**128 - 1,
src_offset_mcopy=2**128 - 1,
dst_offset_mcopy=0,
)
@example(
memory_init_state=b"a" * 100,
size_mcopy=2**128 - 1,
src_offset_mcopy=0,
dst_offset_mcopy=2**128 - 1,
)
def test_should_fail_if_memory_expansion_too_large(
self,
cairo_run,
memory_init_state,
size_mcopy,
src_offset_mcopy,
dst_offset_mcopy,
):
(evm, memory) = cairo_run(
"test__exec_mcopy",
memory_init_state=memory_init_state,
size_mcopy=int_to_uint256(size_mcopy),
src_offset_mcopy=int_to_uint256(src_offset_mcopy),
dst_offset_mcopy=int_to_uint256(dst_offset_mcopy),
)
assert evm["reverted"] == 2
assert b"Kakarot: outOfGas left" in bytes(evm["return_data"])

0 comments on commit 791f1a4

Please sign in to comment.