code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/L2EthToken.sol#L54 https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/L2EthToken.sol#L40-L66 https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/L2EthToken.sol#L40-L76

Vulnerability details

Impact

If the amount of the sender or receiver is greater than or less than the maximum or minimum value of an uint256, the attacker may also result in a loss of cash, the transferFromTo() and mint() functions, which can only be invoked by trusted system contracts, are impacted by the issue.

Proof of Concept

Integer overflow and underflow issues are disregarded using the unchecked keyword, the balance[_to] will overflow and wrap around to "0" if it exceeds the uint256's maximum value, which could allow an attacker to transfer more tokens than was intended. The balance[_to] will similarly underflow and wrap around to the maximum value if it drops below the uint256's minimum value, which might allow an attacker to transfer a negative number of tokens.

A block that modifies the sender and recipient's balances in an uncontrolled manner is the source of the vulnerability in the transferFromTo() and mint() functions. Unexpected behavior may occur if the sender's or recipient's balance is greater than or less than the maximum or minimum value of an uint256 due to an integer overflow or underflow.

By moving a large number of tokens from an account with a balance that is just below the maximum value of uint256, we can change the transferFromTo() function to simulate an integer overflow in order to show this issue. https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/L2EthToken.sol#L40-L66

function transferFromTo(address _from, address _to, uint256 _amount) external override {
    require(
        msg.sender == MSG_VALUE_SYSTEM_CONTRACT ||
            msg.sender == address(DEPLOYER_SYSTEM_CONTRACT) ||
            msg.sender == BOOTLOADER_FORMAL_ADDRESS,
        "Only system contracts with special access can call this method"
    );

    uint256 fromBalance = balance[_from];
    require(fromBalance >= _amount, "Transfer amount exceeds balance");
    unchecked {
        balance[_from] = fromBalance - _amount;
        balance[_to] += _amount;
    }

    emit Transfer(_from, _to, _amount);
}

// Attacker transfers a large amount of tokens from an account close to the maximum value of uint256
function attackTransfer() public {
    uint256 amount = 2 ** 255; // maximum value of uint256 is 2^256-1, so this value is close to the maximum
    transferFromTo(msg.sender, attacker, amount);
}

A significant number of tokens are transferred from an account with a balance that is almost as high as the maximum value of "uint256" in this modified function by an attacker, an integer overflow happens when this function is invoked because the sender's balance is more than the "uint256's" maximum value. This unanticipated behavior causes the recipient's balance to increase to the maximum value of "uint256" while the sender's balance decreases to zero.

By minting a large number of tokens to an account with a balance that is almost equal to the maximum value of "uint256", we may also change the mint() method to imitate an integer overflow. https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/L2EthToken.sol#L72-L76

function mint(address _account, uint256 _amount) external override onlyBootloader {
    totalSupply += _amount;
    balance[_account] += _amount;
    emit Mint(_account, _amount);
}

// Attacker mints a large amount of tokens to an account close to the maximum value of uint256
function attackMint() public {
    uint256 amount = 2 ** 255; // maximum value of uint256 is 2^256-1, so this value is close to the maximum
    mint(attacker, amount);
}

An attacker mints a lot of tokens in this modified function and deposits them into an account with a balance that is almost as high as uint256's maximum value. An integer overflow happens when this function is invoked because the recipient's balance is more than the "uint256" maximum. This unexpected action causes both the recipient's balance and the total supply of tokens to be zero.

Tools Used

Manual audit

Recommended Mitigation Steps

Before making any balance modifications, the contract should be modified to check for integer overflow and underflow, "SafeMath" library or comparable techniques should be used to perform the check. The contract should also be audited to make sure that none of the other functions have similar vulnerabilities.

Affected Code: https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/L2EthToken.sol#L40-L76

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid

GalloDaSballo commented 1 year ago

code-423n4 / 2023-03-zksync-findings