Skip to content

Commit

Permalink
fix: overflow check memory operations charge gas
Browse files Browse the repository at this point in the history
  • Loading branch information
obatirou committed Sep 3, 2024
1 parent f1e7ebb commit 4c69cd5
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 27 deletions.
55 changes: 35 additions & 20 deletions src/kakarot/instructions/environmental_information.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,18 @@ namespace EnvironmentalInformation {
// Any size upper than 2**128 will cause an OOG error, considering the maximum gas for a transaction.
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 size_high_is_zero = Helpers.is_zero(size.high);
local gas_to_charge: felt;
if (size_high_is_zero != FALSE) {
assert gas_to_charge = memory_expansion.cost + copy_gas_cost;
} else {
assert gas_to_charge = Gas.MEMORY_COST_U128;
}

let evm = EVM.charge_gas(evm, gas_to_charge);

// static cost handled in jump table
let evm = EVM.charge_gas(
evm, memory_expansion.cost + copy_gas_cost_low + copy_gas_cost_high
);
if (evm.reverted != FALSE) {
return evm;
}
Expand Down Expand Up @@ -259,13 +264,18 @@ namespace EnvironmentalInformation {
// Any size upper than 2**128 will cause an OOG error, considering the maximum gas for a transaction.
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 size_high_is_zero = Helpers.is_zero(size.high);
local gas_to_charge: felt;
if (size_high_is_zero != FALSE) {
assert gas_to_charge = memory_expansion.cost + copy_gas_cost;
} else {
assert gas_to_charge = Gas.MEMORY_COST_U128;
}

let evm = EVM.charge_gas(evm, gas_to_charge);

// static cost handled in jump table
let evm = EVM.charge_gas(
evm, memory_expansion.cost + copy_gas_cost_low + copy_gas_cost_high
);
if (evm.reverted != FALSE) {
return evm;
}
Expand Down Expand Up @@ -393,19 +403,24 @@ namespace EnvironmentalInformation {
tempvar access_gas_cost = is_warm * Gas.WARM_ACCESS + (1 - is_warm) *
Gas.COLD_ACCOUNT_ACCESS;

let memory_expansion = Gas.memory_expansion_cost_saturated(
memory.words_len, dest_offset, size
);
// Any size upper than 2**128 will cause an OOG error, considering the maximum gas for a transaction.
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 size_high_is_zero = Helpers.is_zero(size.high);
local gas_to_charge: felt;
if (size_high_is_zero != FALSE) {
assert gas_to_charge = memory_expansion.cost + copy_gas_cost + access_gas_cost;
} else {
assert gas_to_charge = Gas.MEMORY_COST_U128;
}

let evm = EVM.charge_gas(evm, gas_to_charge);

let evm = EVM.charge_gas(
evm, access_gas_cost + copy_gas_cost_low + copy_gas_cost_high + memory_expansion.cost
);
if (evm.reverted != FALSE) {
return evm;
}
Expand Down
18 changes: 12 additions & 6 deletions src/kakarot/instructions/memory_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from starkware.cairo.common.alloc import alloc
from starkware.cairo.common.bool import FALSE, TRUE
from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin
from starkware.cairo.common.uint256 import Uint256
from starkware.cairo.common.uint256 import Uint256, felt_to_uint256, uint256_add
from starkware.cairo.common.math_cmp import is_nn, is_not_zero

from kakarot.errors import Errors
Expand Down Expand Up @@ -110,12 +110,18 @@ namespace MemoryOperations {
// Any size upper than 2**128 will cause an OOG error, considering the maximum gas for a transaction.
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 size_high_is_zero = Helpers.is_zero(size.high);
local gas_to_charge: felt;
if (size_high_is_zero != FALSE) {
assert gas_to_charge = memory_expansion.cost + copy_gas_cost;
} else {
assert gas_to_charge = Gas.MEMORY_COST_U128;
}

let evm = EVM.charge_gas(evm, gas_to_charge);

let evm = EVM.charge_gas(
evm, memory_expansion.cost + copy_gas_cost_low + copy_gas_cost_high
);
if (evm.reverted != FALSE) {
return evm;
}
Expand Down
71 changes: 70 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,73 @@ 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_should_copy_a_value_from_memory{
syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
}() {
alloc_locals;
let evm = TestHelpers.init_evm();
let stack = Stack.init();
let state = State.init();
let memory = Memory.init();
// Stack items for mcopy
tempvar size_mcopy = new Uint256(0x20, 0);
tempvar offset_mcopy = new Uint256(0, 0);
tempvar dst_offset_mcopy = new Uint256(0x20, 0);
// Stack items for mstore
tempvar value_mstore = new Uint256(0x1, 0);
tempvar dst_offset_mstore = new Uint256(0, 0);

with stack, memory, state {
// store 1 at offset 0
Stack.push(value_mstore);
Stack.push(dst_offset_mstore);
let evm = MemoryOperations.exec_mstore(evm);
// copy 1 from offset 0 to offset 0x20
Stack.push(size_mcopy);
Stack.push(offset_mcopy);
Stack.push(dst_offset_mcopy);
let evm = MemoryOperations.exec_mcopy(evm);
// load from offset 0x20
Stack.push(dst_offset_mcopy);
let evm = MemoryOperations.exec_mload(evm);
let (index0) = Stack.peek(0);
}
assert stack.size = 1;
assert_uint256_eq([index0], Uint256(0x1, 0));
return ();
}

func test__exec_mcopy_should_fail_if_memory_expansion_to_large{
syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
}() -> model.EVM* {
alloc_locals;
let evm = TestHelpers.init_evm();
let stack = Stack.init();
let state = State.init();
let memory = Memory.init();
// Stack items for mcopy
tempvar size_mcopy = new Uint256(0x20, 0);
tempvar offset_mcopy = new Uint256(0, 0);
tempvar dst_offset_mcopy = new Uint256(0, 1);
// Stack items for mstore
tempvar value_mstore = new Uint256(0x1, 0);
tempvar dst_offset_mstore = new Uint256(0, 0);

with stack, memory, state {
// store 1 at offset 0
Stack.push(value_mstore);
Stack.push(dst_offset_mstore);
let evm = MemoryOperations.exec_mstore(evm);
// copy 1 from offset 0 to offset 0x20
Stack.push(size_mcopy);
Stack.push(offset_mcopy);
Stack.push(dst_offset_mcopy);
let evm = MemoryOperations.exec_mcopy(evm);
}
return evm;
}
9 changes: 9 additions & 0 deletions tests/src/kakarot/instructions/test_memory_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,12 @@ 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:
def test_should_copy_a_value_from_memory(self, cairo_run):
cairo_run("test__exec_mcopy_should_copy_a_value_from_memory")

def test_should_fail_if_memory_expansion_to_large(self, cairo_run):
evm = cairo_run("test__exec_mcopy_should_fail_if_memory_expansion_to_large")
assert evm["reverted"] == 2
assert b"Kakarot: outOfGas left" in bytes(evm["return_data"])

0 comments on commit 4c69cd5

Please sign in to comment.