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: Stack Modification During RETURN Operation #1224

Closed
mhoste51 opened this issue Nov 21, 2024 · 0 comments · Fixed by #1248
Closed

[SECURITY] Non-Compliance: Stack Modification During RETURN Operation #1224

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

Comments

@mhoste51
Copy link

Our team at FuzzingLabs discovered an non-compliance, during the execution of the RETURN operation in Lambdaclass's EVM implementation, an additional value (1) is pushed onto the stack. This behavior deviates from the expected behavior as outlined in the Ethereum Yellow Paper.

Root cause

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

        let gas_cost =
            current_call_frame
                .memory
                .expansion_cost(offset.checked_add(size).ok_or(VMError::Internal(
                    InternalError::ArithmeticOperationOverflow,
                ))?)?;

        self.increase_consumed_gas(current_call_frame, gas_cost)?;

        let return_data = current_call_frame.memory.load_range(offset, size)?.into();
        current_call_frame.returndata = return_data;
        current_call_frame
            .stack
            .push(U256::from(SUCCESS_FOR_RETURN))?;

        Ok(OpcodeSuccess::Result(ResultReason::Return))
    }

The code responsible for this behavior is as follows:

current_call_frame
    .stack
    .push(U256::from(SUCCESS_FOR_RETURN))?;

Here, SUCCESS_FOR_RETURN is a constant defined as 1. This results in the stack being modified during the RETURN operation, with the value 1 added to it.

Impact

  • Non-compliance with the Ethereum Specification:
    • The Yellow Paper does not mention any modification of the stack during the RETURN operation.
  • Interoperability Issues:
    • Smart contracts may exhibit unintended behavior when deployed on Lambdaclass compared to other EVMs, as the additional stack value could lead to unexpected results.
  • Compatibility Problems:
    • Testing or migrating contracts between Lambdaclass and other clients (e.g., REVM, Geth) may lead to inconsistent results.
@maximopalopoli maximopalopoli self-assigned this Nov 25, 2024
@ilitteri ilitteri added the levm Lambda EVM implementation label Nov 25, 2024
github-merge-queue bot pushed a commit that referenced this issue Nov 25, 2024
**Motivation**

Fixes a bug found by [FuzzingLabs](https://github.com/FuzzingLabs) in
return opcode implementation.

**Description**

Now return opcode does not pushes 1 in the stack in case of success,
since it is not the documented behavior.

Closes #1224
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.

3 participants