From 25a58e88bf04de2c40928c7b0d291ad6587f32d1 Mon Sep 17 00:00:00 2001 From: Oba Date: Thu, 5 Sep 2024 17:16:21 +0200 Subject: [PATCH] fix: ensure values < RC_BOUND for charge gas (#1390) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Time spent on this PR: ## Pull request type 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? Resolves #1388 ## What is the new behavior? Ensures values < RC_BOUND for charge gas - - - This change is [Reviewable](https://reviewable.io/reviews/kkrt-labs/kakarot/1390) --- kakarot_scripts/constants.py | 1 - src/kakarot/evm.cairo | 14 +++++++ src/kakarot/gas.cairo | 21 ++++++---- .../environmental_information.cairo | 15 +++++++ .../instructions/logging_operations.cairo | 5 +++ .../instructions/memory_operations.cairo | 20 +++++++++ src/kakarot/instructions/sha3.cairo | 4 ++ .../instructions/system_operations.cairo | 32 +++++++++++++++ .../test_environmental_information.cairo | 8 ++-- .../test_environmental_information.py | 36 +++++++++++++--- .../instructions/test_memory_operations.cairo | 29 +++++++++++++ .../instructions/test_memory_operations.py | 41 +++++++++++++++++++ tests/src/kakarot/test_gas.py | 4 +- tests/utils/helpers.cairo | 3 ++ 14 files changed, 213 insertions(+), 20 deletions(-) diff --git a/kakarot_scripts/constants.py b/kakarot_scripts/constants.py index 530620874..72b363408 100644 --- a/kakarot_scripts/constants.py +++ b/kakarot_scripts/constants.py @@ -19,7 +19,6 @@ logger.setLevel(logging.INFO) load_dotenv() -# Hardcode block gas limit to 7M BLOCK_GAS_LIMIT = 7_000_000 DEFAULT_GAS_PRICE = int(1e9) BEACON_ROOT_ADDRESS = "0x000F3df6D732807Ef1319fB7B8bB8522d0Beac02" diff --git a/src/kakarot/evm.cairo b/src/kakarot/evm.cairo index 1cc11c787..e492251da 100644 --- a/src/kakarot/evm.cairo +++ b/src/kakarot/evm.cairo @@ -102,6 +102,20 @@ namespace EVM { ); } + func out_of_gas{range_check_ptr}(self: model.EVM*, amount: felt) -> model.EVM* { + let (revert_reason_len, revert_reason) = Errors.outOfGas(self.gas_left, amount); + return new model.EVM( + message=self.message, + return_data_len=revert_reason_len, + return_data=revert_reason, + program_counter=self.program_counter, + stopped=TRUE, + gas_left=0, + gas_refund=self.gas_refund, + reverted=Errors.EXCEPTIONAL_HALT, + ); + } + // @notice Update the return data of the current execution context. // @param self The pointer to the execution context. // @param return_data_len The length of the return_data. diff --git a/src/kakarot/gas.cairo b/src/kakarot/gas.cairo index 762e331fc..fa8e9b442 100644 --- a/src/kakarot/gas.cairo +++ b/src/kakarot/gas.cairo @@ -50,11 +50,11 @@ namespace Gas { const COLD_ACCOUNT_ACCESS = 2600; const WARM_ACCESS = 100; const INIT_CODE_WORD_COST = 2; - const MEMORY_COST_U128 = 0x200000000000000000000000000018000000000000000000000000000000; const TX_BASE_COST = 21000; const TX_ACCESS_LIST_ADDRESS_COST = 2400; const TX_ACCESS_LIST_STORAGE_KEY_COST = 1900; const BLOBHASH = 3; + const MEMORY_COST_U32 = 0x200018000000; // @notice Compute the cost of the memory for a given words length. // @dev To avoid range_check overflow, we compute words_len / 512 @@ -115,13 +115,13 @@ namespace Gas { return expansion; } - let is_low_part_overflowing = is_le_felt(2 ** 128, offset.low + size.low); - if (offset.high == 0 and size.high == 0 and is_low_part_overflowing == 0) { + let (q, _) = unsigned_div_rem(offset.low + size.low, 2 ** 32); + if (offset.high == 0 and size.high == 0 and q == 0) { return calculate_gas_extend_memory(words_len, offset.low + size.low); } - // Hardcoded value of cost(2**128) and size of 2**128 bytes = 2**123 words of 32 bytes + // Hardcoded value of cost(2**32) and size of 2**32 bytes = 2**27 words of 32 bytes // This offset would produce an OOG error in any case - let expansion = model.MemoryExpansion(cost=MEMORY_COST_U128, new_words_len=2 ** 123); + let expansion = model.MemoryExpansion(cost=MEMORY_COST_U32, new_words_len=2 ** 27); return expansion; } @@ -150,6 +150,7 @@ namespace Gas { tempvar is_not_saturated = Helpers.is_zero(offset_1.high) * Helpers.is_zero(size_1.high) * Helpers.is_zero(offset_2.high) * Helpers.is_zero(size_2.high); tempvar is_saturated = 1 - is_not_saturated; + tempvar range_check_ptr = range_check_ptr; jmp expansion_cost_saturated if is_saturated != 0; let max_offset_1 = (1 - is_zero_1) * (offset_1.low + size_1.low); @@ -157,6 +158,10 @@ namespace Gas { let max_expansion_is_2 = is_le_felt(max_offset_1, max_offset_2); let max_offset = max_offset_1 * (1 - max_expansion_is_2) + max_offset_2 * max_expansion_is_2; + let (q, _) = unsigned_div_rem(max_offset, 2 ** 32); + tempvar range_check_ptr = range_check_ptr; + jmp expansion_cost_saturated if q != 0; + let expansion = calculate_gas_extend_memory(words_len, max_offset); let expansion = model.MemoryExpansion( cost=expansion.cost, new_words_len=expansion.new_words_len @@ -169,10 +174,10 @@ namespace Gas { return expansion; expansion_cost_saturated: - let range_check_ptr = [fp - 8]; - // Hardcoded value of cost(2**128) and size of 2**128 bytes = 2**123 words of 32 bytes + let range_check_ptr = [ap - 1]; + // Hardcoded value of cost(2**32) and size of 2**32 bytes = 2**27 words of 32 bytes // This offset would produce an OOG error in any case - let expansion = model.MemoryExpansion(cost=Gas.MEMORY_COST_U128, new_words_len=2 ** 123); + let expansion = model.MemoryExpansion(cost=MEMORY_COST_U32, new_words_len=2 ** 27); return expansion; } diff --git a/src/kakarot/instructions/environmental_information.cairo b/src/kakarot/instructions/environmental_information.cairo index f98905b5a..208d424a7 100644 --- a/src/kakarot/instructions/environmental_information.cairo +++ b/src/kakarot/instructions/environmental_information.cairo @@ -175,6 +175,11 @@ namespace EnvironmentalInformation { memory.words_len, memory_offset, size ); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + // 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; @@ -253,6 +258,11 @@ namespace EnvironmentalInformation { memory.words_len, dest_offset, size ); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + // 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; @@ -398,6 +408,11 @@ namespace EnvironmentalInformation { memory.words_len, dest_offset, size ); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + let evm = EVM.charge_gas(evm, access_gas_cost + copy_gas_cost + memory_expansion.cost); if (evm.reverted != FALSE) { return evm; diff --git a/src/kakarot/instructions/logging_operations.cairo b/src/kakarot/instructions/logging_operations.cairo index 559bb2a7d..e94f227e8 100644 --- a/src/kakarot/instructions/logging_operations.cairo +++ b/src/kakarot/instructions/logging_operations.cairo @@ -53,6 +53,11 @@ namespace LoggingOperations { let memory_expansion = Gas.memory_expansion_cost_saturated(memory.words_len, offset, size); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + let size_cost_low = Gas.LOG_DATA * size.low; tempvar size_cost_high = is_not_zero(size.high) * 2 ** 128; let topics_cost = Gas.LOG_TOPIC * topics_len; diff --git a/src/kakarot/instructions/memory_operations.cairo b/src/kakarot/instructions/memory_operations.cairo index cba6309ad..2335f19fa 100644 --- a/src/kakarot/instructions/memory_operations.cairo +++ b/src/kakarot/instructions/memory_operations.cairo @@ -37,6 +37,12 @@ namespace MemoryOperations { let memory_expansion = Gas.memory_expansion_cost_saturated( memory.words_len, [offset], Uint256(32, 0) ); + + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + let evm = EVM.charge_gas(evm, memory_expansion.cost); if (evm.reverted != FALSE) { return evm; @@ -72,6 +78,11 @@ namespace MemoryOperations { let memory_expansion = Gas.memory_expansion_cost_saturated( memory.words_len, offset, Uint256(32, 0) ); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + let evm = EVM.charge_gas(evm, memory_expansion.cost); if (evm.reverted != FALSE) { return evm; @@ -107,6 +118,11 @@ namespace MemoryOperations { memory.words_len, src, size, dst, size ); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + // 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; @@ -258,6 +274,10 @@ namespace MemoryOperations { let memory_expansion = Gas.memory_expansion_cost_saturated( memory.words_len, offset, Uint256(1, 0) ); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } let evm = EVM.charge_gas(evm, memory_expansion.cost); if (evm.reverted != FALSE) { return evm; diff --git a/src/kakarot/instructions/sha3.cairo b/src/kakarot/instructions/sha3.cairo index f44bb07cb..b66d43ac1 100644 --- a/src/kakarot/instructions/sha3.cairo +++ b/src/kakarot/instructions/sha3.cairo @@ -37,6 +37,10 @@ namespace Sha3 { // GAS let memory_expansion = Gas.memory_expansion_cost_saturated(memory.words_len, offset, size); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } let (words, _) = unsigned_div_rem(size.low + 31, 32); let words_gas_cost_low = Gas.KECCAK256_WORD * words; tempvar words_gas_cost_high = is_not_zero(size.high) * 2 ** 128; diff --git a/src/kakarot/instructions/system_operations.cairo b/src/kakarot/instructions/system_operations.cairo index db4512e77..0bbe8e7ad 100644 --- a/src/kakarot/instructions/system_operations.cairo +++ b/src/kakarot/instructions/system_operations.cairo @@ -63,6 +63,10 @@ namespace SystemOperations { // + init_code_gas // + is_create2 * GAS_KECCAK256_WORD * call_data_words let memory_expansion = Gas.memory_expansion_cost_saturated(memory.words_len, offset, size); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } let (calldata_words, _) = unsigned_div_rem(size.low + 31, 32); let init_code_gas_low = Gas.INIT_CODE_WORD_COST * calldata_words; tempvar init_code_gas_high = is_not_zero(size.high) * 2 ** 128; @@ -253,6 +257,10 @@ namespace SystemOperations { let size = popped[1]; let memory_expansion = Gas.memory_expansion_cost_saturated(memory.words_len, offset, size); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } let evm = EVM.charge_gas(evm, memory_expansion.cost); if (evm.reverted != FALSE) { return evm; @@ -295,6 +303,10 @@ namespace SystemOperations { let size = popped[1]; let memory_expansion = Gas.memory_expansion_cost_saturated(memory.words_len, offset, size); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } let evm = EVM.charge_gas(evm, memory_expansion.cost); if (evm.reverted != FALSE) { return evm; @@ -354,6 +366,11 @@ namespace SystemOperations { memory.words_len, args_offset, args_size, ret_offset, ret_size ); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + // Access gas cost. The account is marked as warm in the `generic_call` function, // which performs a `get_account`. let is_account_warm = State.is_account_warm(to); @@ -483,6 +500,11 @@ namespace SystemOperations { memory.words_len, args_offset, args_size, ret_offset, ret_size ); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + // Access gas cost. The account is marked as warm in the `is_account_alive` instruction, // which performs a `get_account`. let is_account_warm = State.is_account_warm(to); @@ -581,6 +603,11 @@ namespace SystemOperations { memory.words_len, args_offset, args_size, ret_offset, ret_size ); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + // Access gas cost. The account is marked as warm in the `is_account_alive` instruction, // which performs a `get_account`. let is_account_warm = State.is_account_warm(code_address); @@ -685,6 +712,11 @@ namespace SystemOperations { memory.words_len, args_offset, args_size, ret_offset, ret_size ); + if (memory_expansion.cost == Gas.MEMORY_COST_U32) { + let evm = EVM.out_of_gas(evm, memory_expansion.cost); + return evm; + } + // Access gas cost. The account is marked as warm in the `generic_call` function, // which performs a `get_account`. let is_account_warm = State.is_account_warm(code_address); diff --git a/tests/src/kakarot/instructions/test_environmental_information.cairo b/tests/src/kakarot/instructions/test_environmental_information.cairo index 19ba82c0a..a0489c5da 100644 --- a/tests/src/kakarot/instructions/test_environmental_information.cairo +++ b/tests/src/kakarot/instructions/test_environmental_information.cairo @@ -151,9 +151,9 @@ func test__exec_extcodecopy_zellic_issue_1258{ return memory; } -func test__exec_codecopy{ +func test__exec_copy{ syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* -}() -> model.Memory* { +}() -> (model.EVM*, model.Memory*) { // Given alloc_locals; local size: felt; @@ -194,10 +194,10 @@ func test__exec_codecopy{ // Then assert stack.size = 0; - return memory; + return (evm, memory); } -func test__exec_codecopy_offset_high_zellic_issue_1258{ +func test__exec_copy_offset_high_zellic_issue_1258{ syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* }() -> model.Memory* { // Given diff --git a/tests/src/kakarot/instructions/test_environmental_information.py b/tests/src/kakarot/instructions/test_environmental_information.py index d8014db5e..da1abcaeb 100644 --- a/tests/src/kakarot/instructions/test_environmental_information.py +++ b/tests/src/kakarot/instructions/test_environmental_information.py @@ -2,6 +2,8 @@ import pytest from Crypto.Hash import keccak +from hypothesis import example, given +from hypothesis import strategies as st from kakarot_scripts.utils.uint256 import int_to_uint256 from tests.utils.syscall_handler import SyscallHandler @@ -145,12 +147,12 @@ class TestCopy: "offset_is_bytecodelen", ], ) - def test_exec_codecopy_should_copy_code( + def test_exec_copy_should_copy_code( self, cairo_run, size, offset, dest_offset, opcode_number, bytecode ): bytecode.insert(0, opcode_number) # random bytecode that can be mutated - memory = cairo_run( - "test__exec_codecopy", + (_, memory) = cairo_run( + "test__exec_copy", size=size, offset=offset, dest_offset=dest_offset, @@ -167,6 +169,30 @@ def test_exec_codecopy_should_copy_code( == copied_bytecode ) + @given( + opcode_number=st.sampled_from([0x39, 0x37]), + offset=st.integers(0, 2**128 - 1), + dest_offset=st.integers(0, 2**128 - 1), + ) + @example(opcode_number=0x39, offset=2**128 - 1, dest_offset=0) + @example(opcode_number=0x39, offset=0, dest_offset=2**128 - 1) + @example(opcode_number=0x37, offset=2**128 - 1, dest_offset=0) + @example(opcode_number=0x37, offset=0, dest_offset=2**128 - 1) + def test_exec_copy_fail_oog( + self, cairo_run, opcode_number, bytecode, offset, dest_offset + ): + bytecode.insert(0, opcode_number) # random bytecode that can be mutated + (evm, _) = cairo_run( + "test__exec_copy", + size=2**128 - 1, + offset=offset, + dest_offset=dest_offset, + bytecode=bytecode, + opcode_number=opcode_number, + ) + assert evm["reverted"] == 2 + assert b"Kakarot: outOfGas left" in bytes(evm["return_data"]) + @pytest.mark.parametrize("opcode_number", [0x39, 0x37]) @pytest.mark.parametrize( "size", @@ -178,13 +204,13 @@ def test_exec_codecopy_should_copy_code( "size_is_0", ], ) - def test_exec_codecopy_offset_high_zellic_issue_1258( + def test_exec_copy_offset_high_zellic_issue_1258( self, cairo_run, size, opcode_number, bytecode ): bytecode.insert(0, opcode_number) # random bytecode that can be mutated offset_high = 1 memory = cairo_run( - "test__exec_codecopy_offset_high_zellic_issue_1258", + "test__exec_copy_offset_high_zellic_issue_1258", size=size, offset_high=offset_high, dest_offset=0, diff --git a/tests/src/kakarot/instructions/test_memory_operations.cairo b/tests/src/kakarot/instructions/test_memory_operations.cairo index 7a0faac65..4d29b7711 100644 --- a/tests/src/kakarot/instructions/test_memory_operations.cairo +++ b/tests/src/kakarot/instructions/test_memory_operations.cairo @@ -204,3 +204,32 @@ func test__exec_mcopy{ } return (evm, memory); } + +func test_exec_mstore{ + syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* +}() -> (model.EVM*, model.Memory*) { + alloc_locals; + let (value_ptr) = alloc(); + let (offset_ptr) = alloc(); + + %{ + segments.write_arg(ids.value_ptr, program_input["value"]) + segments.write_arg(ids.offset_ptr, program_input["offset"]) + %} + + let evm = TestHelpers.init_evm(); + let stack = Stack.init(); + let state = State.init(); + let memory = Memory.init(); + + let value = cast(value_ptr, Uint256*); + let offset = cast(offset_ptr, Uint256*); + + with stack, memory, state { + Stack.push(value); + Stack.push(offset); + + let evm = MemoryOperations.exec_mstore(evm); + } + return (evm, memory); +} diff --git a/tests/src/kakarot/instructions/test_memory_operations.py b/tests/src/kakarot/instructions/test_memory_operations.py index 3a461ef4a..b3406b1cb 100644 --- a/tests/src/kakarot/instructions/test_memory_operations.py +++ b/tests/src/kakarot/instructions/test_memory_operations.py @@ -106,3 +106,44 @@ def test_should_fail_if_memory_expansion_too_large( ) assert evm["reverted"] == 2 assert b"Kakarot: outOfGas left" in bytes(evm["return_data"]) + + class TestMstore: + + @given( + value=integers(min_value=0, max_value=2**256 - 1), + offset=integers(min_value=0, max_value=0xFFFF), + ) + def test_exec_mstore_should_store_a_value_in_memory( + self, cairo_run, value, offset + ): + (evm, memory) = cairo_run( + "test_exec_mstore", + value=int_to_uint256(value), + offset=int_to_uint256(offset), + ) + + expected_memory = ( + int.to_bytes( + value, length=(value.bit_length() + 7) // 8, byteorder="big" + ) + if value > 0 + else b"\x00" + ) + assert bytes.fromhex(memory)[offset : offset + 32] == bytes( + [0] * (32 - len(expected_memory)) + list(expected_memory) + ) + + @given( + value=integers(min_value=0, max_value=2**256 - 1), + offset=integers(min_value=2**32, max_value=2**256 - 1), + ) + def test_exec_mstore_should_fail_if_memory_expansion_too_large( + self, cairo_run, value, offset + ): + (evm, _) = cairo_run( + "test_exec_mstore", + value=int_to_uint256(value), + offset=int_to_uint256(offset), + ) + assert evm["reverted"] == 2 + assert b"Kakarot: outOfGas left" in bytes(evm["return_data"]) diff --git a/tests/src/kakarot/test_gas.py b/tests/src/kakarot/test_gas.py index 17c420b09..8b69d5173 100644 --- a/tests/src/kakarot/test_gas.py +++ b/tests/src/kakarot/test_gas.py @@ -73,8 +73,8 @@ def test_memory_expansion_cost_saturated(self, cairo_run, offset, size): ) if size == 0: cost = 0 - elif offset + size > 2**128 - 1: - cost = 0x200000000000000000000000000018000000000000000000000000000000 + elif offset + size > 2**32: + cost = calculate_memory_gas_cost(2**32) else: cost = calculate_gas_extend_memory(b"", [(offset, size)]).cost diff --git a/tests/utils/helpers.cairo b/tests/utils/helpers.cairo index 847061924..9e1891d8a 100644 --- a/tests/utils/helpers.cairo +++ b/tests/utils/helpers.cairo @@ -20,6 +20,7 @@ from kakarot.library import Kakarot from kakarot.memory import Memory from kakarot.model import model from kakarot.stack import Stack +from utils.maths import unsigned_div_rem from utils.utils import Helpers namespace TestHelpers { @@ -119,6 +120,8 @@ namespace TestHelpers { ) -> model.Memory* { alloc_locals; let memory = Memory.init(); + let (words_len, _) = unsigned_div_rem(serialized_memory_len + 31, 32); + tempvar memory = new model.Memory(memory.word_dict_start, memory.word_dict, words_len); with memory { Memory.store_n(serialized_memory_len, serialized_memory, 0); }