Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SECURITY] Non-Compliance in CALLDATACOPY: Incomplete Memory Resizing #1247

Closed
mhoste51 opened this issue Nov 25, 2024 · 0 comments · Fixed by #1256
Closed

[SECURITY] Non-Compliance in CALLDATACOPY: Incomplete Memory Resizing #1247

mhoste51 opened this issue Nov 25, 2024 · 0 comments · Fixed by #1256
Assignees
Labels
levm Lambda EVM implementation

Comments

@mhoste51
Copy link

Our team at FuzzingLabs discovered a new non-compliance in the implementation of the CALLDATACOPY opcode, the resizing of memory does not take into account the entire size specified in the operation. Instead, the memory is only resized for the first 32 bytes of data, regardless of whether the size parameter exceeds this length. This leads to incorrect memory management and an invalid MSIZE value.

Root cause

    // CALLDATACOPY operation
    pub fn op_calldatacopy(
        &mut self,
        current_call_frame: &mut CallFrame,
    ) -> Result<OpcodeSuccess, VMError> {
        let dest_offset: usize = current_call_frame
            .stack
            .pop()?
            .try_into()
            .map_err(|_err| VMError::VeryLargeNumber)?;
        let calldata_offset: usize = current_call_frame
            .stack
            .pop()?
            .try_into()
            .map_err(|_err| VMError::VeryLargeNumber)?;
        let size: usize = current_call_frame
            .stack
            .pop()?
            .try_into()
            .map_err(|_err| VMError::VeryLargeNumber)?;

        let gas_cost = gas_cost::calldatacopy(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 mut data = [0u8; 32];
        for (i, byte) in current_call_frame
            .calldata
            .iter()
            .skip(calldata_offset)
            .take(32)
            .enumerate()
        {
            if let Some(data_byte) = data.get_mut(i) {
                *data_byte = *byte;
            }
        }

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

        Ok(OpcodeSuccess::Continue)
    }

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

let mut data = [0u8; 32];
for (i, byte) in current_call_frame
    .calldata
    .iter()
    .skip(calldata_offset)
    .take(32)
    .enumerate()
{
    if let Some(data_byte) = data.get_mut(i) {
        *data_byte = *byte;
    }
}

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

When size = 34, for example, the memory is resized only to accommodate 32 bytes ([0; 32]). This means that any subsequent memory accesses outside this range will not reflect the expected size, and MSIZE will return 32 instead of 64.

The CALLDATACOPY opcode should resize memory to account for the full size specified in the stack input. If size = 34, the memory should be resized to at least 64 bytes to ensure alignment with the EVM specification. This should be calculated like this :

((!size + 1) & 31) + size

Example

  • When size = 34:

    • !size + 1 = 0xFFFFFFFF...FFFD + 1 = 0xFFFFFFFF...FFFE (Two's complement).
    • (0xFFFFFFFF...FFFE & 31) = 30 (Padding required to reach the next multiple of 32).
    • aligned_size = 30 + 34 = 64 (Final aligned size).

    Explanation: The original size of 34 is not a multiple of 32, so the nearest multiple is 64. This padding ensures the memory is correctly aligned.

  • When size = 32:

    • !size + 1 = 0xFFFFFFFF...FFE0 + 1 = 0xFFFFFFFF...FFE1.
    • (0xFFFFFFFF...FFE1 & 31) = 0 (No padding required).
    • aligned_size = 0 + 32 = 32.

    Explanation: Since the original size is already a multiple of 32, no additional padding is needed.

Step to reproduce

Payload

[0x60, 34, 0x5f, 0x5f, 55, 89]
PUSH1 34
PUSH0
PUSH0
CALLDATACOPY
MSIZE

Add to test :

#[test]
fn test_non_compliance_calldatacopy_memory_resize() {
    let mut vm = new_vm_with_bytecode(Bytes::copy_from_slice(&[0x60, 34, 0x5f, 0x5f, 55, 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(64));
}

Backtrace

thread 'test_non_compliance_calldatacopy_memory_resize' panicked at crates/vm/levm/tests/edge_case_tests.rs:18:5:
assertion `left == right` failed
  left: 32
 right: 64
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_calldatacopy_memory_resize
             at ./tests/edge_case_tests.rs:18:5
   5: edge_case_tests::test_non_compliance_calldatacopy_memory_resize::{{closure}}
             at ./tests/edge_case_tests.rs:11:52
   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_calldatacopy_memory_resize

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 17 filtered out; finished in 0.10s
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
levm Lambda EVM implementation
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants