Delegatecall Behavioral Inconsistency with SHA256 Precompile

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/contracts/precompiles/SHA256.yul#L9

Vulnerability details

Impact

The behavioral discrepancy in zkSync Era when using delegatecall with the SHA256 precompile contract can have significant impacts:

1. Security Risk: It introduces a potential security risk, as delegatecall may not produce the expected results, affecting the integrity of data and contracts.
2. Compatibility Concerns: Developers transitioning to zkSync may encounter compatibility issues, necessitating careful handling of precompile contracts.

Proof of Concept

It is observed that in the zkSync Era, there is a distinct inconsistency in the behavior of the SHA256 precompile contract (located at address 0x02) when accessed through a delegatecall. This behavior deviates from the standard Ethereum Virtual Machine (EVM) behavior, where the results across call, staticcall, and delegatecall are consistent.
https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/contracts/precompiles/SHA256.yul#L9

Within the zkSync Era, invoking the SHA256 precompile contract using a delegatecall takes a different path from the usual behavior. Instead of executing within the isolated context of the called contract, it delegates the call to the contract itself and runs its code within the caller's context. Consequently, the returned value is not in line with the expected outcome of a precompileCall. Instead, it consistently yields bytes32(0).

For clarity, when executing the provided code below in the EVM, the returned bytes32 value remains constant at 0x66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925 for all three scenarios: sha256Staticcall, sha256Call, and sha256Delegatecall. However, in the zkSync Era, while sha256Staticcall and sha256Call deliver results consistent with the EVM, sha256Delegatecall produces an inaccurate outcome.

// SPDX-License-Identifier: MIT

pragma solidity >=0.8.20;

contract PoC {
    function sha256Staticcall() public returns (bytes32) {
        assembly {
            pop(staticcall(gas(), 0x02, 0, 0x20, 0, 0x20))
            return(0, 0x20)
        }
    }

    function sha256Call() public returns (bytes32) {
        assembly {
            pop(call(gas(), 0x02, 0, 0, 0x20, 0, 0x20))
            return(0, 0x20)
        }
    }

    function sha256Delegatecall() public returns (bytes32) {
        assembly {
            pop(delegatecall(gas(), 0x02, 0, 0x20, 0, 0x20))
            return(0, 0x20)
        }
    }
}

This discrepancy is critical in its impact because it introduces a divergence from the expected EVM response. While the likelihood of encountering this issue is not high, as precompile contracts are typically invoked through staticcall rather than delegatecall.

Tools Used

Recommended Mitigation Steps

It is advisable to substitute a delegatecall with a staticcall whenever the target address for the operation is the SHA256 precompiled contract.

function delegateCall(
        uint256 _gas,
        address _address,
        bytes calldata _data
    ) internal returns (bytes memory returnData) {
        bool success;
        if(_address == address(0x02){
            success = rawStaticCall(_gas, _address, _data);
        } else {
            success = rawDelegateCall(_gas, _address, _data);
        }
        returnData = _verifyCallResult(success);
    }

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/contracts/libraries/EfficientCall.sol#L88

Assessed type

Context

[c4-pre-sort]

bytes032 marked the issue as primary issue

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[miladpiri]

High impact, low probablity ⇒ medium

[c4-sponsor]

miladpiri (sponsor) confirmed

[GalloDaSballo]

The Warden has shown an inconsistency in the behaviour of sha256 when using delegatecall

Because the goal of the zkSync EVM is to be the EVM compatible, Medium Severity seems appropriate

[c4-judge]

GalloDaSballo marked the issue as selected for report

[GalloDaSballo]

I have verified my statement through testing via the zksync-foundry code repo as well as reviewing both the Precompile as well as the Rust code

[GalloDaSballo]

This finding is valid and the behaviour is inconsistent, Medium Severity is appropriate

[c4-judge]

GalloDaSballo marked the issue as duplicate of #175

[c4-judge]

GalloDaSballo marked the issue as not selected for report

[c4-judge]

GalloDaSballo marked the issue as satisfactory