Skip to content

Commit

Permalink
fix: saturate memory offset taken from stack (#950)
Browse files Browse the repository at this point in the history 
* fix: saturate memory offset taken from stack

* fix tests

* fmt

* fix test'
  • Loading branch information
@enitrat
enitrat committed Sep 17, 2024
1 parent 08311ef commit fed4554
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 99 deletions.
2 changes: 1 addition & 1 deletion crates/evm/src/create_helpers.cairo
View file
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
View file
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
138 changes: 60 additions & 78 deletions crates/evm/src/instructions/logging_operations.cairo
View file
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 Expand Up @@ -107,18 +107,18 @@ mod tests {
let result = vm.exec_log0();

// Then
assert(result.is_ok(), 'should have succeeded');
assert(vm.stack.len() == 0, 'stack should be empty');
assert!(result.is_ok());
assert_eq!(vm.stack.len(), 0);

let mut events = vm.env.state.events;
assert(events.len() == 1, 'context should have one event');
assert_eq!(events.len(), 1);

let event = events.pop_front().unwrap();
assert(event.keys.len() == 0, 'event should not have keys');
assert_eq!(event.keys.len(), 0);

assert(event.data.len() == 31, 'event should have 31 bytes');
assert_eq!(event.data.len(), 31);
let data_expected = u256_to_bytes_array(Bounded::<u256>::MAX).span().slice(0, 31);
assert(event.data.span() == data_expected, 'event data are incorrect');
assert_eq!(event.data.span(), data_expected);
}

#[test]
Expand All @@ -136,19 +136,19 @@ mod tests {
let result = vm.exec_log1();

// Then
assert(result.is_ok(), 'should have succeeded');
assert(vm.stack.len() == 0, 'stack should be empty');
assert!(result.is_ok());
assert_eq!(vm.stack.len(), 0);

let mut events = vm.env.state.events;
assert(events.len() == 1, 'context should have one event');
assert_eq!(events.len(), 1);

let event = events.pop_front().unwrap();
assert(event.keys.len() == 1, 'event should have one key');
assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct');
assert_eq!(event.keys.len(), 1);
assert_eq!(event.keys.span(), [0x0123456789ABCDEF].span());

assert(event.data.len() == 32, 'event should have 32 bytes');
assert_eq!(event.data.len(), 32);
let data_expected = u256_to_bytes_array(Bounded::<u256>::MAX).span().slice(0, 32);
assert(event.data.span() == data_expected, 'event data are incorrect');
assert_eq!(event.data.span(), data_expected);
}

#[test]
Expand All @@ -167,20 +167,19 @@ mod tests {
let result = vm.exec_log2();

// Then
assert(result.is_ok(), 'should have succeeded');
assert(vm.stack.len() == 0, 'stack should be empty');
assert!(result.is_ok());
assert_eq!(vm.stack.len(), 0);

let mut events = vm.env.state.events;
assert(events.len() == 1, 'context should have one event');
assert_eq!(events.len(), 1);

let event = events.pop_front().unwrap();
assert(event.keys.len() == 2, 'event should have two keys');
assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct');
assert(*event.keys[1] == Bounded::<u256>::MAX, 'event key is not correct');
assert_eq!(event.keys.len(), 2);
assert_eq!(event.keys.span(), [0x0123456789ABCDEF, Bounded::<u256>::MAX].span());

assert(event.data.len() == 5, 'event should have 5 bytes');
assert_eq!(event.data.len(), 5);
let data_expected = u256_to_bytes_array(Bounded::<u256>::MAX).span().slice(0, 5);
assert(event.data.span() == data_expected, 'event data are incorrect');
assert_eq!(event.data.span(), data_expected);
}

#[test]
Expand All @@ -205,23 +204,20 @@ mod tests {
let result = vm.exec_log3();

// Then
assert(result.is_ok(), 'should have succeeded');
assert(vm.stack.len() == 0, 'stack should be empty');
assert!(result.is_ok());
assert_eq!(vm.stack.len(), 0);

let mut events = vm.env.state.events;
assert(events.len() == 1, 'context should have one event');
assert_eq!(events.len(), 1);

let event = events.pop_front().unwrap();
assert(event.keys.len() == 3, 'event should have 3 keys');
assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct');
assert(*event.keys[1] == Bounded::<u256>::MAX, 'event key is not correct');
assert(*event.keys[2] == 0x00, 'event key is not correct');
assert_eq!(event.keys.span(), [0x0123456789ABCDEF, Bounded::<u256>::MAX, 0x00].span());

assert(event.data.len() == 40, 'event should have 40 bytes');
assert_eq!(event.data.len(), 40);
let data_expected = u256_to_bytes_array(Bounded::<u256>::MAX).span();
assert(event.data.span().slice(0, 32) == data_expected, 'event data are incorrect');
assert_eq!(event.data.span().slice(0, 32), data_expected);
let data_expected = [0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF].span();
assert(event.data.span().slice(32, 8) == data_expected, 'event data are incorrect');
assert_eq!(event.data.span().slice(32, 8), data_expected);
}

#[test]
Expand All @@ -247,22 +243,21 @@ mod tests {
let result = vm.exec_log4();

// Then
assert(result.is_ok(), 'should have succeeded');
assert(vm.stack.len() == 0, 'stack should be empty');
assert!(result.is_ok());
assert_eq!(vm.stack.len(), 0);

let mut events = vm.env.state.events;
assert(events.len() == 1, 'context should have one event');
assert_eq!(events.len(), 1);

let event = events.pop_front().unwrap();
assert(event.keys.len() == 4, 'event should have 4 keys');
assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct');
assert(*event.keys[1] == Bounded::<u256>::MAX, 'event key is not correct');
assert(*event.keys[2] == 0x00, 'event key is not correct');
assert(*event.keys[3] == Bounded::<u256>::MAX, 'event key is not correct');
assert_eq!(
event.keys.span(),
[0x0123456789ABCDEF, Bounded::<u256>::MAX, 0x00, Bounded::<u256>::MAX].span()
);

assert(event.data.len() == 10, 'event should have 10 bytes');
assert_eq!(event.data.len(), 10);
let data_expected = [0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x00, 0x00].span();
assert(event.data.span() == data_expected, 'event data are incorrect');
assert_eq!(event.data.span(), data_expected);
}

#[test]
Expand All @@ -280,10 +275,8 @@ mod tests {
let result = vm.exec_log1();

// Then
assert(result.is_err(), 'should have returned an error');
assert(
result.unwrap_err() == EVMError::WriteInStaticContext, 'err != WriteInStaticContext'
);
assert!(result.is_err());
assert_eq!(result.unwrap_err(), EVMError::WriteInStaticContext);
}

#[test]
Expand All @@ -301,17 +294,16 @@ mod tests {
let result = vm.exec_log1();

// Then
assert(result.is_ok(), 'should have succeeded');
assert(vm.stack.len() == 0, 'stack should be empty');
assert!(result.is_ok());
assert_eq!(vm.stack.len(), 0);

let mut events = vm.env.state.events;
assert(events.len() == 1, 'context should have one event');
assert_eq!(events.len(), 1);

let event = events.pop_front().unwrap();
assert(event.keys.len() == 1, 'event should have one key');
assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct');
assert_eq!(event.keys.span(), [0x0123456789ABCDEF].span());

assert(event.data.len() == 0, 'event data should be empty');
assert_eq!(event.data.len(), 0);
}

#[test]
Expand All @@ -329,11 +321,8 @@ mod tests {
let result = vm.exec_log1();

// Then
assert(result.is_err(), 'should return an error');
assert(
result.unwrap_err() == EVMError::TypeConversionError(TYPE_CONVERSION_ERROR),
'err != TypeConversionError'
);
assert!(result.is_err());
assert_eq!(result.unwrap_err(), EVMError::TypeConversionError(TYPE_CONVERSION_ERROR));
}

#[test]
Expand All @@ -351,11 +340,8 @@ mod tests {
let result = vm.exec_log1();

// Then
assert(result.is_err(), 'should return an error');
assert(
result.unwrap_err() == EVMError::TypeConversionError(TYPE_CONVERSION_ERROR),
'err != TypeConversionError'
);
assert!(result.is_err());
assert_eq!(result.unwrap_err(), EVMError::MemoryLimitOOG);
}

#[test]
Expand Down Expand Up @@ -387,32 +373,28 @@ mod tests {
vm.exec_log4().expect('exec_log4 failed');

// Then
assert(vm.stack.len() == 0, 'stack size should be 0');
assert!(vm.stack.is_empty());

let mut events = vm.env.state.events;
assert(events.len() == 2, 'context should have 2 events');
assert_eq!(events.len(), 2);

let event1 = events.pop_front().unwrap();
assert(event1.keys.len() == 3, 'event1 should have 3 keys');
assert(*event1.keys[0] == 0x0123456789ABCDEF, 'event1 key is not correct');
assert(*event1.keys[1] == Bounded::<u256>::MAX, 'event1 key is not correct');
assert(*event1.keys[2] == 0x00, 'event1 key is not correct');
assert_eq!(event1.keys.span(), [0x0123456789ABCDEF, Bounded::<u256>::MAX, 0x00].span());

assert(event1.data.len() == 40, 'event1 should have 40 bytes');
assert_eq!(event1.data.len(), 40);
let data_expected = u256_to_bytes_array(Bounded::<u256>::MAX).span();
assert(event1.data.span().slice(0, 32) == data_expected, 'event1 data are incorrect');
assert_eq!(event1.data.span().slice(0, 32), data_expected);
let data_expected = [0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF].span();
assert(event1.data.span().slice(32, 8) == data_expected, 'event1 data are incorrect');
assert_eq!(event1.data.span().slice(32, 8), data_expected);

let event2 = events.pop_front().unwrap();
assert(event2.keys.len() == 4, 'event2 should have 4 keys');
assert(*event2.keys[0] == 0x0123456789ABCDEF, 'event2 key is not correct');
assert(*event2.keys[1] == Bounded::<u256>::MAX, 'event2 key is not correct');
assert(*event2.keys[2] == 0x00, 'event2 key is not correct');
assert(*event2.keys[3] == Bounded::<u256>::MAX, 'event2 key is not correct');
assert_eq!(
event2.keys.span(),
[0x0123456789ABCDEF, Bounded::<u256>::MAX, 0x00, Bounded::<u256>::MAX].span()
);

assert(event2.data.len() == 10, 'event2 should have 10 bytes');
assert_eq!(event2.data.len(), 10);
let data_expected = [0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x00, 0x00].span();
assert(event2.data.span() == data_expected, 'event2 data are incorrect');
assert_eq!(event2.data.span(), data_expected);
}
}
4 changes: 2 additions & 2 deletions crates/evm/src/instructions/memory_operations.cairo
View file
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
View file
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
Loading

0 comments on commit fed4554

Please sign in to comment.