code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-zksync//blob/main/contracts/L2EthToken.sol#L78-L94

Vulnerability details

Summary

// https://github.com/code-423n4/2023-03-zksync//blob/main/contracts/L2EthToken.sol#L78-L94
    /// @notice Initiate the ETH withdrawal, funds will be available to claim on L1 `finalizeEthWithdrawal` method.
    /// @param _l1Receiver The address on L1 to receive the funds.
    function withdraw(address _l1Receiver) external payable override {
        uint256 amount = msg.value;

        // Silent burning of the ether
        unchecked {
            balance[address(this)] -= amount;
            totalSupply -= amount;
        }

        // Send the L2 log, a user could use it as proof of the withdrawal
        bytes memory message = _getL1WithdrawMessage(_l1Receiver, amount);
        L1_MESSENGER_CONTRACT.sendToL1(message);

        emit Withdrawal(msg.sender, _l1Receiver, amount);
    }

msg.value is externally controllable and depends on the amount paid by the caller. When msg.value is greater than balance[address(this)], integer overflow will occur. Similarly, totalSupply may also be affected.

Impact

As a result, balance[address(this)] and totalSupply become super large, which may be combined with other vulnerabilities.

Proof of Concept

Tools Used

Manual

Recommended Mitigation Steps

Do not use the unchecked modifier.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #110