Skip to content

Commit

Permalink
fix: saturate memory offset taken from stack
Browse files Browse the repository at this point in the history
  • Loading branch information
enitrat committed Sep 16, 2024
1 parent 9024471 commit cd0bffd
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 22 deletions.
2 changes: 1 addition & 1 deletion crates/evm/src/create_helpers.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub impl CreateHelpersImpl of CreateHelpers {
/// As part of the CREATE family of opcodes.
fn prepare_create(ref self: VM, create_type: CreateType) -> Result<CreateArgs, EVMError> {
let value = self.stack.pop()?;
let offset = self.stack.pop_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;

let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span());
Expand Down
16 changes: 8 additions & 8 deletions crates/evm/src/instructions/environmental_information.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
/// Save word to memory.
/// # Specification: https://www.evm.codes/#37?fork=shanghai
fn exec_calldatacopy(ref self: VM) -> Result<(), EVMError> {
let dest_offset = self.stack.pop_usize()?;
let offset = self.stack.pop_usize()?;
let dest_offset = self.stack.pop_saturating_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;

let words_size = (ceil32(size) / 32).into();
Expand Down Expand Up @@ -141,8 +141,8 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
/// Copies slice of bytecode to memory.
/// # Specification: https://www.evm.codes/#39?fork=shanghai
fn exec_codecopy(ref self: VM) -> Result<(), EVMError> {
let dest_offset = self.stack.pop_usize()?;
let offset = self.stack.pop_usize()?;
let dest_offset = self.stack.pop_saturating_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;

let words_size = (ceil32(size) / 32).into();
Expand Down Expand Up @@ -190,8 +190,8 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
/// # Specification: https://www.evm.codes/#3c?fork=shanghai
fn exec_extcodecopy(ref self: VM) -> Result<(), EVMError> {
let evm_address = self.stack.pop_eth_address()?;
let dest_offset = self.stack.pop_usize()?;
let offset = self.stack.pop_usize()?;
let dest_offset = self.stack.pop_saturating_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;

// GAS
Expand Down Expand Up @@ -227,8 +227,8 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
/// Save word to memory.
/// # Specification: https://www.evm.codes/#3e?fork=shanghai
fn exec_returndatacopy(ref self: VM) -> Result<(), EVMError> {
let dest_offset = self.stack.pop_usize()?;
let offset = self.stack.pop_usize()?;
let dest_offset = self.stack.pop_saturating_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let return_data: Span<u8> = self.return_data();

Expand Down
2 changes: 1 addition & 1 deletion crates/evm/src/instructions/logging_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ fn exec_log_i(ref self: VM, topics_len: u8) -> Result<(), EVMError> {
ensure(!self.message().read_only, EVMError::WriteInStaticContext)?;

// TODO(optimization): check benefits of n `pop` instead of `pop_n`
let offset = self.stack.pop_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let topics: Array<u256> = self.stack.pop_n(topics_len.into())?;

Expand Down
4 changes: 2 additions & 2 deletions crates/evm/src/instructions/memory_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub impl MemoryOperation of MemoryOperationTrait {
/// Save single byte to memory
/// # Specification: https://www.evm.codes/#53?fork=shanghai
fn exec_mstore8(ref self: VM) -> Result<(), EVMError> {
let offset = self.stack.pop_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let value = self.stack.pop()?;
let value: u8 = (value.low & 0xFF).try_into().unwrap();

Expand Down Expand Up @@ -282,7 +282,7 @@ pub impl MemoryOperation of MemoryOperationTrait {
/// Copy memory from one location to another.
/// # Specification: https://www.evm.codes/#5e?fork=cancun
fn exec_mcopy(ref self: VM) -> Result<(), EVMError> {
let dest_offset = self.stack.pop_usize()?;
let dest_offset = self.stack.pop_saturating_usize()?;
let source_offset = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?;

Expand Down
20 changes: 10 additions & 10 deletions crates/evm/src/instructions/system_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ pub impl SystemOperations of SystemOperationsTrait {
let gas = self.stack.pop_saturating_u64()?;
let to = self.stack.pop_eth_address()?;
let value = self.stack.pop()?;
let args_offset = self.stack.pop_usize()?;
let args_offset = self.stack.pop_saturating_usize()?;
let args_size = self.stack.pop_usize()?;
let ret_offset = self.stack.pop_usize()?;
let ret_offset = self.stack.pop_saturating_usize()?;
let ret_size = self.stack.pop_usize()?;

// GAS
Expand Down Expand Up @@ -107,9 +107,9 @@ pub impl SystemOperations of SystemOperationsTrait {
let gas = self.stack.pop_saturating_u64()?;
let code_address = self.stack.pop_eth_address()?;
let value = self.stack.pop()?;
let args_offset = self.stack.pop_usize()?;
let args_offset = self.stack.pop_saturating_usize()?;
let args_size = self.stack.pop_usize()?;
let ret_offset = self.stack.pop_usize()?;
let ret_offset = self.stack.pop_saturating_usize()?;
let ret_size = self.stack.pop_usize()?;

let to = self.message().target.evm;
Expand Down Expand Up @@ -170,7 +170,7 @@ pub impl SystemOperations of SystemOperationsTrait {
/// RETURN
/// # Specification: https://www.evm.codes/#f3?fork=shanghai
fn exec_return(ref self: VM) -> Result<(), EVMError> {
let offset = self.stack.pop_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span());
self.memory.ensure_length(memory_expansion.new_size);
Expand All @@ -191,9 +191,9 @@ pub impl SystemOperations of SystemOperationsTrait {
fn exec_delegatecall(ref self: VM) -> Result<(), EVMError> {
let gas = self.stack.pop_saturating_u64()?;
let code_address = self.stack.pop_eth_address()?;
let args_offset = self.stack.pop_usize()?;
let args_offset = self.stack.pop_saturating_usize()?;
let args_size = self.stack.pop_usize()?;
let ret_offset = self.stack.pop_usize()?;
let ret_offset = self.stack.pop_saturating_usize()?;
let ret_size = self.stack.pop_usize()?;

// GAS
Expand Down Expand Up @@ -244,9 +244,9 @@ pub impl SystemOperations of SystemOperationsTrait {
fn exec_staticcall(ref self: VM) -> Result<(), EVMError> {
let gas = self.stack.pop_saturating_u64()?;
let to = self.stack.pop_eth_address()?;
let args_offset = self.stack.pop_usize()?;
let args_offset = self.stack.pop_saturating_usize()?;
let args_size = self.stack.pop_usize()?;
let ret_offset = self.stack.pop_usize()?;
let ret_offset = self.stack.pop_saturating_usize()?;
let ret_size = self.stack.pop_usize()?;

// GAS
Expand Down Expand Up @@ -287,7 +287,7 @@ pub impl SystemOperations of SystemOperationsTrait {
/// REVERT
/// # Specification: https://www.evm.codes/#fd?fork=shanghai
fn exec_revert(ref self: VM) -> Result<(), EVMError> {
let offset = self.stack.pop_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;

let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span());
Expand Down
39 changes: 39 additions & 0 deletions crates/evm/src/stack.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub trait StackTrait {
fn push(ref self: Stack, item: u256) -> Result<(), EVMError>;
fn pop(ref self: Stack) -> Result<u256, EVMError>;
fn pop_usize(ref self: Stack) -> Result<usize, EVMError>;
fn pop_saturating_usize(ref self: Stack) -> Result<usize, EVMError>;
fn pop_u64(ref self: Stack) -> Result<u64, EVMError>;
fn pop_saturating_u64(ref self: Stack) -> Result<u64, EVMError>;
fn pop_u128(ref self: Stack) -> Result<u128, EVMError>;
Expand Down Expand Up @@ -107,6 +108,19 @@ impl StackImpl of StackTrait {
Result::Ok(item)
}

/// Calls `Stack::pop` and saturates the result to usize
#[inline(always)]
fn pop_saturating_usize(ref self: Stack) -> Result<usize, EVMError> {
let item: u256 = self.pop()?;
if item.high != 0 {
return Result::Ok(Bounded::<usize>::MAX);
};
match item.low.try_into() {
Option::Some(value) => Result::Ok(value),
Option::None => Result::Ok(Bounded::<usize>::MAX),
}
}

/// Calls `Stack::pop` and tries to convert it to u64
///
/// # Errors
Expand Down Expand Up @@ -420,6 +434,31 @@ mod tests {
);
}

#[test]
fn test_pop_saturating_usize_should_return_max_when_overflow() {
// Given
let mut stack = StackTrait::new();
stack.push(Bounded::<u64>::MAX.into()).unwrap();

// When
let result = stack.pop_saturating_usize();
assert!(result.is_ok());
assert_eq!(result.unwrap(), Bounded::<usize>::MAX);
}

#[test]
fn test_pop_saturating_usize_should_return_value_when_no_overflow() {
// Given
let mut stack = StackTrait::new();
stack.push(1234567890).unwrap();

// When
let result = stack.pop_saturating_usize();
assert!(result.is_ok());
assert_eq!(result.unwrap(), 1234567890);
}


#[test]
fn test_pop_saturating_u64_should_return_max_when_overflow() {
// Given
Expand Down

0 comments on commit cd0bffd

Please sign in to comment.