Unchecked return value of call will allow to send messages marked as sent but will fail due to not enough gas

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/L1Messenger.sol#L49

Vulnerability details

Impact

On the sendTol1 function, they are sending the message via the SystemContractHelper:

SystemContractHelper.toL1(true, bytes32(uint256(uint160(msg.sender))), hash);

the problem relies on the fact that they are not checking whether the message was actually sent.

    function toL1(bool _isService, bytes32 _key, bytes32 _value) internal {
    address callAddr = TO_L1_CALL_ADDRESS;
    assembly {
        // Ensuring that the type is bool
        _isService := and(_isService, 1)
        // This `success` is always 0, but the method always succeeds
        // (except for the cases when there is not enough gas)
        let success := call(_isService, callAddr, _key, _value, 0xFFFF, 0, 0)
    }

let success := call(_isService, callAddr, _key, _value, 0xFFFF, 0, 0)

There is no handling return of call. Anyone can send a message with less than 0xFFFF gas and the message will directly fail to deliver marking it as sent:

     emit L1MessageSent(msg.sender, hash, _message);

Proof of Concept

interface NoGas{
function sendToL1(bytes calldata _message) external ;
 }

contract BrickMessage{

function noMessage()external{
    NoGas(0xdD870fA1b7C4700F2BD7f44238821C26f7392148).sendToL1{gas:50000}("0x0");
}

  }

Tools Used

Manual

Recommended Mitigation Steps

Require that the call was successful

(bool suc,) = SystemContractHelper.toL1(true, bytes32(uint256(uint160(msg.sender))), hash);

require(succ, "wrong");

[GalloDaSballo]

Similar to #209 but not the same part of the code

[miladpiri]

It is not an actual call, but an opcode simulation by the compiler, so if it fails with gas, the frame will fail as well.

[c4-sponsor]

miladpiri marked the issue as sponsor disputed

[GalloDaSballo]

@miladpiri can you please clarify your statement concerning the frame failing?

In reading the source code call seems to receive "custom parameters", the document: https://github.com/code-423n4/2023-03-zksync/blob/main/docs/VM-specific_v1.3.0_opcodes_simulation.pdf

Clarifies that's because of a custom usage of that opcode

Meaning that the call in toL1 is translated to:

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/bootloader/bootloader.yul#L2407-L2409

            function sendToL1(isService, key, value) {
                verbatim_3i_0o("to_l1", isService, key, value)
            } 

Meaning that call cannot run out of gas because the call in the code doesn't actually correspond to the "conventional" usage of the call opcode.

TL;DR It cannot run out of gas because it always has all the gas available since it's a simulation

Am I missing something or does the above sum up your argument?

[miladpiri]

@GalloDaSballo You are true. The behavior is different from conventional call.

[GalloDaSballo]

Per discussion with the sponsor, the compiler changes the bytecode from the "conventional" call to a custom implementation, explained above

Given the fact that gas is always provided, and that the warden failed to explain the nuance of the zkCompiler, I must close as invalid

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid