Description

The EVM circuits ErrorOOGMemoryCopyGadget gadget is used to constrain reverts due to running out of gas on a memory copy operation. Let us consider this snippet from ErrorOOGMemoryCopyGadget::configure in zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs:

let dst_memory_addr = MemoryExpandedAddressGadget::construct_self(cb);
// src can also be possible to overflow for mcopy.
let src_memory_addr = MemoryExpandedAddressGadget::construct_self(cb);

cb.stack_pop(dst_memory_addr.offset_rlc());
cb.stack_pop(src_memory_addr.offset_rlc());
cb.stack_pop(dst_memory_addr.length_rlc());

// ...

cb.require_equal(
    // for mcopy, both dst_memory_addr and dst_memory_addr likely overflow.
    "Memory address is overflow or gas left is less than cost",
    or::expr([
        dst_memory_addr.overflow(),
        src_memory_addr.overflow(),
        insufficient_gas.expr(),
    ]),
    1.expr(),
);

The length for src_memory_addr is never constrained, so an attacker can set it to anything. The implementation for MemoryExpandedAddressGadget::overflow and the relevant constraints are as follows.

/// Check if overflow.
pub(crate) fn overflow(&self) -> Expression<F> {
    not::expr(self.within_range())
}

/// Check if within range.
pub(crate) fn within_range(&self) -> Expression<F> {
    or::expr([
        self.length_is_zero.expr(),
        and::expr([
            self.sum_lt_cap.expr(),
            self.sum_within_u64.expr(),
            not::expr(self.offset_length_sum.carry().as_ref().unwrap()),
        ]),
    ])
}

fn construct_self(cb: &mut EVMConstraintBuilder<F>) -> Self {
    let offset = cb.query_word_rlc();
    let length = cb.query_word_rlc();
    let sum = cb.query_word_rlc();

    let sum_lt_cap = LtGadget::construct(
        cb,
        from_bytes::expr(&sum.cells[..N_BYTES_U64]),
        (MAX_EXPANDED_MEMORY_ADDRESS + 1).expr(),
    );

    let sum_overflow_hi = sum::expr(&sum.cells[N_BYTES_U64..]);
    let sum_within_u64 = IsZeroGadget::construct(cb, sum_overflow_hi);

    let length_is_zero = IsZeroGadget::construct(cb, sum::expr(&length.cells));
    let offset_length_sum = AddWordsGadget::construct(cb, [offset, length], sum);

    Self {
        length_is_zero,
        offset_length_sum,
        sum_lt_cap,
        sum_within_u64,
    }
}

When length can be set to an arbitrary value, an arbitrary value for sum = offset + length can be arranged, in particular a value larger than $2^{64}-1$. Thus, it can be arranged that sum_within_u64 is false, making within_range return false, and thus making overflow return true.

This means an attacker can prove an execution where arbitrary MCOPYs of their choice incorrectly fail due to being out of gas.

Impact

An attacker can prove execution of a transaction where MCOPYs of their choice might incorrectly revert. There are some scenarios in which an attacker can profit from such reverts. One scenario would be in cases where a caller wishes to use a feature the callee may or may not support, so the caller wraps the call in a try block, falling back to some default behavior in the case that the call reverted. Some examples of this are here↗ and here↗. Another scenario would be cases in which something negative for user A is caused by a transaction submitted by a third party B, for example in the case of liquidation of collaterals for a loan or opening of a commitment that shows that user A lost a bet.

Recommendations

Constrain the length used in src_memory_addr to be equal to the length in dst_memory_addr (which is in turn already constrained to be the length popped from the stack).

Remediation

This issue has been acknowledged by Scroll, and a fix was implemented in commit 929589e2↗.