code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/BytecodeCompressor.sol#L62 https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/L1Messenger.sol#L49 https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/libraries/SystemContractHelper.sol#L48

Vulnerability details

Impact

Users can use BytecodeCompressor.publishCompressedBytecode() to publish compressed bytecode on the L1. However, L1Messenger.sendToL1 uses msg.sender as sender in the L2->L1 log. In other words, the sender of all the compressed bytecodes published through BytecodeCompressor.publishCompressedBytecode() is BytecodeCompressor instead of the real sender.

Proof of Concept

When a user calls BytecodeCompressor.publishCompressedBytecode(), it would call L1_MESSENGER_CONTRACT.sendToL1(_rawCompressedData); https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/BytecodeCompressor.sol#L62

    function publishCompressedBytecode(
        bytes calldata _bytecode,
        bytes calldata _rawCompressedData
    ) external payable returns (bytes32 bytecodeHash) {
        …

        bytes32 rawCompressedDataHash = L1_MESSENGER_CONTRACT.sendToL1(_rawCompressedData);
        …

Then, L1Messenger.sendToL1 calls SystemContractHelper.toL1 using msg.sender as the sender of the message. https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/L1Messenger.sol#L49

    function sendToL1(bytes calldata _message) external override returns (bytes32 hash) {
        …

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

        …
    }

Then the sender of the compressed bytecode becomes BytecodeCompressor.

Tools Used

Manual Review

Recommended Mitigation Steps

There could be multiple methods to fix the issue. One is adding a new function in L1Messenger. The function takes a custom sender. And the function can be only called by BytecodeCompressor

    function sendToL1(bytes calldata _message, uint160 sender) external override returns (bytes32 hash) {
        require(msg.sender == address(BYTECODE_COMPRESSOR_CONTRACT), "Callable only by the bytecode compressor");
        …

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

        …
    }