[SECURITY] Non-Compliance : Memory Alignment in EXTCODECOPY Operation

[mhoste51]

Our team at FuzzingLabs discovered a new non-compliance in the current implementation of the op_extcodecopy function in levm, the allocated memory size during the execution of the EXTCODECOPY opcode is not aligned to a multiple of 32 bytes, which is the expected behavior as per EVM standards. For instance, if a size of size = 12 is specified, the function allocates memory of exactly 12 bytes instead of rounding up to 32 bytes.

Root cause

    // EXTCODECOPY operation
    pub fn op_extcodecopy(
        &mut self,
        current_call_frame: &mut CallFrame,
    ) -> Result<OpcodeSuccess, VMError> {
        let address = word_to_address(current_call_frame.stack.pop()?);
        let dest_offset: usize = current_call_frame
            .stack
            .pop()?
            .try_into()
            .map_err(|_| VMError::VeryLargeNumber)?;
        let offset: usize = current_call_frame
            .stack
            .pop()?
            .try_into()
            .map_err(|_| VMError::VeryLargeNumber)?;
        let size: usize = current_call_frame
            .stack
            .pop()?
            .try_into()
            .map_err(|_| VMError::VeryLargeNumber)?;

        let is_cached = self.cache.is_account_cached(&address);

        let gas_cost = gas_cost::extcodecopy(current_call_frame, size, dest_offset, is_cached)
            .map_err(VMError::OutOfGas)?;

        self.increase_consumed_gas(current_call_frame, gas_cost)?;

        if !is_cached {
            self.cache_from_db(&address);
        };

        let bytecode = self.get_account(&address).info.bytecode;

        let new_memory_size = dest_offset.checked_add(size).ok_or(VMError::Internal(
            InternalError::ArithmeticOperationOverflow,
        ))?;
        let current_memory_size = current_call_frame.memory.data.len();
        if current_memory_size < new_memory_size {
            current_call_frame.memory.data.resize(new_memory_size, 0);
        }

        for i in 0..size {
            if let Some(memory_byte) =
                current_call_frame
                    .memory
                    .data
                    .get_mut(dest_offset.checked_add(i).ok_or(VMError::Internal(
                        InternalError::ArithmeticOperationOverflow,
                    ))?)
            {
                *memory_byte = *bytecode
                    .get(offset.checked_add(i).ok_or(VMError::Internal(
                        InternalError::ArithmeticOperationOverflow,
                    ))?)
                    .unwrap_or(&0u8);
            }
        }

        Ok(OpcodeSuccess::Continue)
    }

Here is the portion of code responsible for this non-compliance:

let new_memory_size = dest_offset.checked_add(size).ok_or(VMError::Internal(
    InternalError::ArithmeticOperationOverflow,
))?;
if current_memory_size < new_memory_size {
    current_call_frame.memory.data.resize(new_memory_size, 0);
}

Memory is resized directly to size without considering 32-byte alignment. For example:

• If size = 12, memory is resized to 12 bytes.
• If size = 64, memory is correctly resized to 64 bytes (already aligned).

However, the EVM specification requires that all memory allocations be aligned to the next multiple of 32 bytes. (explained in this issue)

Step to reproduce

Payload

[0x60, 12, 0x5f, 0x5f, 0x5f, 0x3c, 89]

PUSH1 12
PUSH0
PUSH0
PUSH0
EXCCODECOPY
MSIZE

Add to test :

#[test]
fn test_non_compliance_extcodecopy_memory_resize() {
    let mut vm = new_vm_with_bytecode(Bytes::copy_from_slice(&[0x60, 12, 0x5f, 0x5f, 0x5f, 0x3c, 89]
    ))
    .unwrap();
    let mut current_call_frame = vm.call_frames.pop().unwrap();
    vm.execute(&mut current_call_frame);
    assert_eq!(current_call_frame.stack.stack[0], U256::from(32));
}

Backtrace

thread 'test_non_compliance_extcodecopy_memory_resize' panicked at crates/vm/levm/tests/edge_case_tests.rs:17:5:
assertion `left == right` failed
  left: 12
 right: 32
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8adb4b30f40e6fbd21dc1ba26c3301c7eeb6de3c/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/8adb4b30f40e6fbd21dc1ba26c3301c7eeb6de3c/library/core/src/panicking.rs:76:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /home/mhoste/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panicking.rs:373:5
   4: edge_case_tests::test_non_compliance_extcodecopy_memory_resize
             at ./tests/edge_case_tests.rs:17:5
   5: edge_case_tests::test_non_compliance_extcodecopy_memory_resize::{{closure}}
             at ./tests/edge_case_tests.rs:11:51
   6: core::ops::function::FnOnce::call_once
             at /home/mhoste/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/8adb4b30f40e6fbd21dc1ba26c3301c7eeb6de3c/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

failures:
    test_non_compliance_extcodecopy_memory_resize

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 18 filtered out; finished in 0.10s