From 86c2cf01935e40649a1f0b6d9ab9f17c51cefe49 Mon Sep 17 00:00:00 2001 From: enitrat Date: Mon, 16 Sep 2024 18:57:04 +0200 Subject: [PATCH] fix: memory expansion cost could overflow --- crates/evm/src/create_helpers.cairo | 2 +- crates/evm/src/errors.cairo | 2 + crates/evm/src/gas.cairo | 42 ++++++++++++------- .../environmental_information.cairo | 8 ++-- .../src/instructions/logging_operations.cairo | 2 +- .../src/instructions/memory_operations.cairo | 9 ++-- crates/evm/src/instructions/sha3.cairo | 2 +- .../src/instructions/system_operations.cairo | 12 +++--- 8 files changed, 48 insertions(+), 31 deletions(-) diff --git a/crates/evm/src/create_helpers.cairo b/crates/evm/src/create_helpers.cairo index 64ff58a16..baf8f0487 100644 --- a/crates/evm/src/create_helpers.cairo +++ b/crates/evm/src/create_helpers.cairo @@ -42,7 +42,7 @@ pub impl CreateHelpersImpl of CreateHelpers { let offset = self.stack.pop_usize()?; let size = self.stack.pop_usize()?; - let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span()); + let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span())?; self.memory.ensure_length(memory_expansion.new_size); let init_code_gas = gas::init_code_cost(size); let charged_gas = match create_type { diff --git a/crates/evm/src/errors.cairo b/crates/evm/src/errors.cairo index 097720215..eab671cad 100644 --- a/crates/evm/src/errors.cairo +++ b/crates/evm/src/errors.cairo @@ -58,6 +58,7 @@ pub enum EVMError { OutOfGas, Assertion, DepthLimit, + MemoryLimitOOG } #[generate_trait] @@ -81,6 +82,7 @@ pub impl EVMErrorImpl of EVMErrorTrait { EVMError::OutOfGas => 'out of gas'.into(), EVMError::Assertion => 'assertion failed'.into(), EVMError::DepthLimit => 'max call depth exceeded'.into(), + EVMError::MemoryLimitOOG => 'memory limit out of gas'.into(), } } diff --git a/crates/evm/src/gas.cairo b/crates/evm/src/gas.cairo index 8819d7d65..77274be0b 100644 --- a/crates/evm/src/gas.cairo +++ b/crates/evm/src/gas.cairo @@ -1,4 +1,6 @@ use core::cmp::min; +use core::num::traits::CheckedAdd; +use evm::errors::EVMError; use utils::eth_transaction::common::TxKindTrait; use utils::eth_transaction::eip2930::{AccessListItem}; use utils::eth_transaction::transaction::{Transaction, TransactionTrait}; @@ -166,30 +168,42 @@ pub fn calculate_memory_gas_cost(size_in_bytes: usize) -> u64 { /// # Returns /// /// * `MemoryExpansion`: New size and expansion cost. -pub fn memory_expansion(current_size: usize, operations: Span<(usize, usize)>) -> MemoryExpansion { - let mut max_size = current_size; - - for ( - offset, size - ) in operations { - if *size != 0 { - let end = *offset + *size; - if end > max_size { - max_size = end; - } +pub fn memory_expansion( + current_size: usize, mut operations: Span<(usize, usize)> +) -> Result { + let mut current_max_size = current_size; + + // Using a high-level loop because Cairo doesn't support the `for` loop syntax with breaks + let max_size = loop { + match operations.pop_front() { + Option::Some(( + offset, size + )) => { + if *size != 0 { + match (*offset).checked_add(*size) { + Option::Some(end) => { + if end > current_max_size { + current_max_size = end; + } + }, + Option::None => { break Result::Err(EVMError::MemoryLimitOOG); }, + } + } + }, + Option::None => { break Result::Ok((current_max_size)); }, } - }; + }?; let new_size = helpers::ceil32(max_size); if new_size <= current_size { - return MemoryExpansion { new_size: current_size, expansion_cost: 0 }; + return Result::Ok(MemoryExpansion { new_size: current_size, expansion_cost: 0 }); } let prev_cost = calculate_memory_gas_cost(current_size); let new_cost = calculate_memory_gas_cost(new_size); let expansion_cost = new_cost - prev_cost; - MemoryExpansion { new_size, expansion_cost } + Result::Ok(MemoryExpansion { new_size, expansion_cost }) } /// Calculates the gas to be charged for the init code in CREATE/CREATE2 diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index 3c65e6238..f6bcd1ca3 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -119,7 +119,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { let copy_gas_cost = gas::COPY * words_size; let memory_expansion = gas::memory_expansion( self.memory.size(), [(dest_offset, size)].span() - ); + )?; self.memory.ensure_length(memory_expansion.new_size); self.charge_gas(gas::VERYLOW + copy_gas_cost + memory_expansion.expansion_cost)?; @@ -149,7 +149,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { let copy_gas_cost = gas::COPY * words_size; let memory_expansion = gas::memory_expansion( self.memory.size(), [(dest_offset, size)].span() - ); + )?; self.memory.ensure_length(memory_expansion.new_size); self.charge_gas(gas::VERYLOW + copy_gas_cost + memory_expansion.expansion_cost)?; @@ -198,7 +198,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { let words_size = (ceil32(size) / 32).into(); let memory_expansion = gas::memory_expansion( self.memory.size(), [(dest_offset, size)].span() - ); + )?; self.memory.ensure_length(memory_expansion.new_size); let copy_gas_cost = gas::COPY * words_size; let access_gas_cost = if self.accessed_addresses.contains(evm_address) { @@ -244,7 +244,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { let memory_expansion = gas::memory_expansion( self.memory.size(), [(dest_offset, size)].span() - ); + )?; self.memory.ensure_length(memory_expansion.new_size); self.charge_gas(gas::VERYLOW + copy_gas_cost + memory_expansion.expansion_cost)?; diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index 86c0422a7..571e15736 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -64,7 +64,7 @@ fn exec_log_i(ref self: VM, topics_len: u8) -> Result<(), EVMError> { let size = self.stack.pop_usize()?; let topics: Array = self.stack.pop_n(topics_len.into())?; - let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span()); + let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span())?; self.memory.ensure_length(memory_expansion.new_size); self .charge_gas( diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index ab6222651..b6c11b0aa 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -38,7 +38,7 @@ pub impl MemoryOperation of MemoryOperationTrait { fn exec_mload(ref self: VM) -> Result<(), EVMError> { let offset: usize = self.stack.pop_usize()?; - let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 32)].span()); + let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 32)].span())?; self.memory.ensure_length(memory_expansion.new_size); self.charge_gas(gas::VERYLOW + memory_expansion.expansion_cost)?; @@ -52,7 +52,7 @@ pub impl MemoryOperation of MemoryOperationTrait { fn exec_mstore(ref self: VM) -> Result<(), EVMError> { let offset: usize = self.stack.pop_usize()?; let value: u256 = self.stack.pop()?; - let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 32)].span()); + let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 32)].span())?; self.memory.ensure_length(memory_expansion.new_size); self.charge_gas(gas::VERYLOW + memory_expansion.expansion_cost)?; @@ -68,7 +68,7 @@ pub impl MemoryOperation of MemoryOperationTrait { let value = self.stack.pop()?; let value: u8 = (value.low & 0xFF).try_into().unwrap(); - let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 1)].span()); + let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 1)].span())?; self.memory.ensure_length(memory_expansion.new_size); self.charge_gas(gas::VERYLOW + memory_expansion.expansion_cost)?; @@ -290,7 +290,7 @@ pub impl MemoryOperation of MemoryOperationTrait { let copy_gas_cost = gas::COPY * words_size; let memory_expansion = gas::memory_expansion( self.memory.size(), [(max(dest_offset, source_offset), size)].span() - ); + )?; self.memory.ensure_length(memory_expansion.new_size); self.charge_gas(gas::VERYLOW + copy_gas_cost + memory_expansion.expansion_cost)?; @@ -1054,6 +1054,7 @@ mod tests { + gas::memory_expansion( vm.memory.size(), [(max(dest_offset, source_offset), size)].span() ) + .unwrap() .expansion_cost + copy_gas_cost; let gas_before = vm.gas_left(); diff --git a/crates/evm/src/instructions/sha3.cairo b/crates/evm/src/instructions/sha3.cairo index 0928f7611..e85bc528c 100644 --- a/crates/evm/src/instructions/sha3.cairo +++ b/crates/evm/src/instructions/sha3.cairo @@ -26,7 +26,7 @@ pub impl Sha3Impl of Sha3Trait { let words_size = (ceil32(size) / 32).into(); let word_gas_cost = gas::KECCAK256WORD * words_size; - let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span()); + let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span())?; self.memory.ensure_length(memory_expansion.new_size); self.charge_gas(gas::KECCAK256 + word_gas_cost + memory_expansion.expansion_cost)?; diff --git a/crates/evm/src/instructions/system_operations.cairo b/crates/evm/src/instructions/system_operations.cairo index 525f71f7e..fb8dfe670 100644 --- a/crates/evm/src/instructions/system_operations.cairo +++ b/crates/evm/src/instructions/system_operations.cairo @@ -36,7 +36,7 @@ pub impl SystemOperations of SystemOperationsTrait { // GAS let memory_expansion = gas::memory_expansion( self.memory.size(), [(args_offset, args_size), (ret_offset, ret_size)].span() - ); + )?; self.memory.ensure_length(memory_expansion.new_size); let access_gas_cost = if self.accessed_addresses.contains(to) { @@ -117,7 +117,7 @@ pub impl SystemOperations of SystemOperationsTrait { // GAS let memory_expansion = gas::memory_expansion( self.memory.size(), [(args_offset, args_size), (ret_offset, ret_size)].span() - ); + )?; self.memory.ensure_length(memory_expansion.new_size); let access_gas_cost = if self.accessed_addresses.contains(code_address) { @@ -172,7 +172,7 @@ pub impl SystemOperations of SystemOperationsTrait { fn exec_return(ref self: VM) -> Result<(), EVMError> { let offset = self.stack.pop_usize()?; let size = self.stack.pop_usize()?; - let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span()); + let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span())?; self.memory.ensure_length(memory_expansion.new_size); self.charge_gas(gas::ZERO + memory_expansion.expansion_cost)?; @@ -199,7 +199,7 @@ pub impl SystemOperations of SystemOperationsTrait { // GAS let memory_expansion = gas::memory_expansion( self.memory.size(), [(args_offset, args_size), (ret_offset, ret_size)].span() - ); + )?; self.memory.ensure_length(memory_expansion.new_size); let access_gas_cost = if self.accessed_addresses.contains(code_address) { @@ -252,7 +252,7 @@ pub impl SystemOperations of SystemOperationsTrait { // GAS let memory_expansion = gas::memory_expansion( self.memory.size(), [(args_offset, args_size), (ret_offset, ret_size)].span() - ); + )?; self.memory.ensure_length(memory_expansion.new_size); let access_gas_cost = if self.accessed_addresses.contains(to) { @@ -290,7 +290,7 @@ pub impl SystemOperations of SystemOperationsTrait { let offset = self.stack.pop_usize()?; let size = self.stack.pop_usize()?; - let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span()); + let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span())?; self.memory.ensure_length(memory_expansion.new_size); self.charge_gas(memory_expansion.expansion_cost)?;