[SECURITY] Non-Compliance in RETURNDATACOPY Implementation

[mhoste51]

Our team at FuzzingLabs discovered a non compliance in RETURNDATACOPY implementation. This implementation diverges from the expected behavior defined in the Ethereum Yellow Paper. Specifically, when the returndata_offset exceeds the length of the returndata, the implementation improperly allocates additional memory filled with zeros instead of returning an Out of Offset error, as required by the specifications.

Root cause

    // RETURNDATACOPY operation
    pub fn op_returndatacopy(
        &mut self,
        current_call_frame: &mut CallFrame,
    ) -> Result<OpcodeSuccess, VMError> {
        let dest_offset: usize = current_call_frame
            .stack
            .pop()?
            .try_into()
            .map_err(|_| VMError::VeryLargeNumber)?;
        let returndata_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 gas_cost = gas_cost::returndatacopy(current_call_frame, size, dest_offset)
            .map_err(VMError::OutOfGas)?;

        self.increase_consumed_gas(current_call_frame, gas_cost)?;

        if size == 0 {
            return Ok(OpcodeSuccess::Continue);
        }

        let sub_return_data_len = current_call_frame.sub_return_data.len();
        let data = if returndata_offset < sub_return_data_len {
            current_call_frame.sub_return_data.slice(
                returndata_offset
                    ..(returndata_offset
                        .checked_add(size)
                        .ok_or(VMError::Internal(
                            InternalError::ArithmeticOperationOverflow,
                        ))?)
                    .min(sub_return_data_len),
            )
        } else {
            vec![0u8; size].into()
        };

        current_call_frame.memory.store_bytes(dest_offset, &data)?;

        Ok(OpcodeSuccess::Continue)
    }

Here is the problematic section in the current implementation:

let data = if returndata_offset < sub_return_data_len {
    current_call_frame.sub_return_data.slice(
        returndata_offset
            ..(returndata_offset
                .checked_add(size)
                .ok_or(VMError::Internal(
                    InternalError::ArithmeticOperationOverflow,
                ))?)
            .min(sub_return_data_len),
    )
} else {
    vec![0u8; size].into()
};

When returndata_offset >= sub_return_data_len (i.e., the offset is out of bounds), the implementation does not fail. Instead, it creates a vector of zeros with a size equal to the requested range:

vec![0u8; size].into()

This behavior is non-compliant because:

1. An error should be raised instead of allocating memory.
2. The RETURNDATACOPY operation should terminate immediately upon detecting an invalid offset (OutOfOffset).

[mhoste51]

test

Here is the test :

#[test]
fn test_non_compliance_returndatacopy() {
    let mut vm = new_vm_with_bytecode(Bytes::copy_from_slice(&[56, 56, 56, 56, 56, 56, 62, 56]))
    .unwrap();
    let mut current_call_frame = vm.call_frames.pop().unwrap();
    let txreport = vm.execute(&mut current_call_frame);
    match txreport.result{
        Success =>  panic!("This should return an error"),
        Revert(e) => {
            assert_eq!(e, VMError::OutOfOffset)
        }
    }
}

Backtrace

thread 'test_non_compliance_returndatacopy' panicked at crates/vm/levm/tests/edge_case_tests.rs:15:21:
This should return an error
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: edge_case_tests::test_non_compliance_returndatacopy
             at ./tests/edge_case_tests.rs:15:21
   3: edge_case_tests::test_non_compliance_returndatacopy::{{closure}}
             at ./tests/edge_case_tests.rs:9:40
   4: 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
   5: 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_returndatacopy

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