Attacker can run malicious code on contract through bad tokens on bridging

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/main/code/contracts/ethereum/contracts/bridge/L1ERC20Bridge.sol#L162 https://github.com/code-423n4/2024-03-zksync/blob/main/code/contracts/ethereum/contracts/bridge/L1ERC20Bridge.sol#L135

Vulnerability details

Impact

• Bridge can be used to bridge spam tokens to users.
• Bridge can be exploited to call bad code leading to loss of funds of users and DOS.

Proof of Concept

A Malicious User can inject bad code using an arbitrary token with bad code called in the _transfer function.

For a rough example a bad token like below could be used on the bridge successfully.

contract MalToken is FeeOnTransferToken {
    address usdc; // Define the address of USDC token

    constructor(
        string memory name, 
        string memory symbol, 
        uint8 decimals,
        address _usdc // Pass the address of USDC token as an argument
    ) FeeOnTransferToken(name, symbol, decimals) {
        usdc = _usdc;
    }

    function _transfer(address from, address to, uint256 amount) internal override {        
        // Approve USDC token for transfer
        FeeOnTransferToken(usdc).approve(address(this), amount);

        // Transfer USDC token from sender to this contract
        FeeOnTransferToken(usdc).transferFrom(msg.sender, address(this), amount);

        // Perform the token transfer
        super._transfer(from, to, amount);
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

• WhiteList tokens in bridge
• Allow only verified clients to bridge allowing user bridge through verified client who are mandated to have whitelisted or supported tokens

Assessed type

ERC20

[saxenism]

Invalid. Non standard implementations of ERC20 are not allowed.

[c4-sponsor]

saxenism (sponsor) disputed

[alex-ppg]

The Warden specifies that a malicious token implementation can compromise the L1ERC20Bridge contract's operations, however, it does not clarify in this regard.

The PoC provided is invalid as the FeeOnTransferToken::approve operation would have a msg.sender of the contract itself rather than the caller. As such, I consider this exhibit to be invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid