c4-bot-3 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/system-contracts/contracts/L1Messenger.sol#L50-L51

Vulnerability details

Impact

The keccakGasCost is not correctly calculated. The keccakGasCost will be greater then expected. This issue is related to user's fund loss - because user - in same scenarios - will pay more gas than expected.

Proof of Concept

File: L1Messenger.sol

    function keccakGasCost(uint256 _length) internal pure returns (uint256) {
        return KECCAK_ROUND_GAS_COST * (_length / KECCAK_ROUND_NUMBER_OF_BYTES + 1);
    }

Let's consider a scenario where _length == KECCAK_ROUND_NUMBER_OF_BYTES.

keccakGasCost(KECCAK_ROUND_NUMBER_OF_BYTES) = KECCAK_ROUND_GAS_COST * (KECCAK_ROUND_NUMBER_OF_BYTES / KECCAK_ROUND_NUMBER_OF_BYTES + 1) = KECCAK_ROUND_GAS_COST * 2. When the _length is KECCAK_ROUND_NUMBER_OF_BYTES, the cost of keccak should actually be KECCAK_ROUND_GAS_COST, however it's twice as big as it should be.

In other words, when _length is in a form of N * KECCAK_ROUND_GAS_COST (_length is divisible by KECCAK_ROUND_GAS_COST), the keccak cost has one additional KECCAK_ROUND_GAS_COST.

For keccakGasCost(N * KECCAK_ROUND_NUMBER_OF_BYTES) the cost should be exactly: N * KECCAK_ROUND_NUMBER_OF_BYTES, however function keccakGasCost() returns: keccakGasCost(N * KECCAK_ROUND_NUMBER_OF_BYTES) = KECCAK_ROUND_GAS_COST * (N * KECCAK_ROUND_NUMBER_OF_BYTES / KECCAK_ROUND_NUMBER_OF_BYTES + 1) = KECCAK_ROUND_GAS_COST * ( N + 1) = N * KECCAK_ROUND_GAS_COST + KECCAK_ROUND_GAS_COST. As we see, there's additional KECCAK_ROUND_GAS_COST.

Tools Used

Manual code review

Recommended Mitigation Steps

When _length == N * KECCAK_ROUND_NUMBER_OF_BYTES, function keccakGasCost() should return N * KECCAK_ROUND_NUMBER_OF_BYTES instead of N * KECCAK_ROUND_NUMBER_OF_BYTES + KECCAK_ROUND_NUMBER_OF_BYTES

Assessed type

Math

c4-sponsor commented 5 months ago

saxenism (sponsor) disputed

saxenism commented 5 months ago

This is considered invalid because this is how keccak256 padding works. If the length is divisible by 136 it adds 136 bytes of padding. Otherwise, it pads the data such that the total length will be divisible.

alex-ppg commented 4 months ago

The Warden specifies that the keccak256 gas cost is incorrectly calculated, however, the calculation accurately represents the Keccak256.yul precompile calculations. As such, this submission is invalid.

c4-judge commented 4 months ago

alex-ppg marked the issue as unsatisfactory: Invalid