sherlock-audit / 2024-08-tokamak-network-judging

1 stars 0 forks source link

eta - Unverified Cross-Chain Token and Messages Allow Manipulation and Financial Loss #55

Open sherlock-admin2 opened 1 month ago

sherlock-admin2 commented 1 month ago

eta

High

Unverified Cross-Chain Token and Messages Allow Manipulation and Financial Loss

Summary

The StandardBridge.sol and CrossDomainMessenger.sol contracts are vulnerable to exploitation due to the lack of verification for the sender of cross-chain messages or tokens. This allows any user to send arbitrary tokens along with fraudulent data to the StandardBridge.sol contract on the destination chain, which is then processed as if it were legitimate.

Vulnerability Detail

L1 and L2 contracts interact with the StandardBridge.sol contract to receive native and ERC20 tokens, as well as cross-chain data. When tokens are bridged, the sendMessage function of the CrossDomainMessenger.sol contract is invoked. The sendMessage function handles the tokens and associated data by calling the finalizeBridgeERC20 function and the relayMessage function.

However, the sendMessage function does not verify the sender of the cross-chain message (or the received tokens).

This allows anyone to send any tokens along with arbitrary data to the StandardBridge.sol contract on the other chain. Then, the relayMessage function in the CrossDomainMessenger.sol contract will process the data, assuming it represents valid staking targets and incentives.

An attacker can bridge any tokens to the StandardBridge.sol contract on the other chain using fake data to manipulate staking incentives. Since the StandardBridge.sol contract does not seem to provide a way to access the original sender’s address on the source chain when executing the receiver callback after receiving the tokens, the most sensible mitigation approach may be to send the bridged tokens and associated staking data separately. tokamak-thanos/packages/tokamak/contracts-bedrock/src/universal/StandardBridge.sol_initiateBridgeETH_L331-L335

    /// @notice Initiates a bridge of ETH through the CrossDomainMessenger.
    /// @param _from        Address of the sender.
    /// @param _to          Address of the receiver.
    /// @param _amount      Amount of ETH being bridged.
    /// @param _minGasLimit Minimum amount of gas that the bridge can be relayed with.
    /// @param _extraData   Extra data to be sent with the transaction. Note that the recipient will
    ///                     not be triggered with this data, but it will be emitted and can be used
    ///                     to identify the transaction.
    function _initiateBridgeETH(
        address _from,
        address _to,
        uint256 _amount,
        uint32 _minGasLimit,
        bytes memory _extraData
    )
        internal
    {
        require(isCustomGasToken() == false, "StandardBridge: cannot bridge ETH with custom gas token");
        require(msg.value == _amount, "StandardBridge: bridging ETH must include sufficient ETH value");

        // Emit the correct events. By default this will be _amount, but child
        // contracts may override this function in order to emit legacy events as well.
        _emitETHBridgeInitiated(_from, _to, _amount, _extraData);

        messenger.sendMessage{ value: _amount }({
            _target: address(otherBridge),
            _message: abi.encodeWithSelector(this.finalizeBridgeETH.selector, _from, _to, _amount, _extraData),
            _minGasLimit: _minGasLimit
        });
    }

tokamak-thanos/packages/tokamak/contracts-bedrock/src/universal/StandardBridge.sol_initiateBridgeERC20_L376-L391

    /// @notice Sends ERC20 tokens to a receiver's address on the other chain.
    /// @param _localToken  Address of the ERC20 on this chain.
    /// @param _remoteToken Address of the corresponding token on the remote chain.
    /// @param _to          Address of the receiver.
    /// @param _amount      Amount of local tokens to deposit.
    /// @param _minGasLimit Minimum amount of gas that the bridge can be relayed with.
    /// @param _extraData   Extra data to be sent with the transaction. Note that the recipient will
    ///                     not be triggered with this data, but it will be emitted and can be used
    ///                     to identify the transaction.
    function _initiateBridgeERC20(
        address _localToken,
        address _remoteToken,
        address _from,
        address _to,
        uint256 _amount,
        uint32 _minGasLimit,
        bytes memory _extraData
    )
        internal
    {
        require(msg.value == 0, "StandardBridge: cannot send value");

        if (_isOptimismMintableERC20(_localToken)) {
            require(
                _isCorrectTokenPair(_localToken, _remoteToken),
                "StandardBridge: wrong remote token for Optimism Mintable ERC20 local token"
            );

            OptimismMintableERC20(_localToken).burn(_from, _amount);
        } else {
            IERC20(_localToken).safeTransferFrom(_from, address(this), _amount);
            deposits[_localToken][_remoteToken] = deposits[_localToken][_remoteToken] + _amount;
        }

        // Emit the correct events. By default this will be ERC20BridgeInitiated, but child
        // contracts may override this function in order to emit legacy events as well.
        _emitERC20BridgeInitiated(_localToken, _remoteToken, _from, _to, _amount, _extraData);

        messenger.sendMessage({
            _target: address(otherBridge),
            _message: abi.encodeWithSelector(
                this.finalizeBridgeERC20.selector,
                // Because this call will be executed on the remote chain, we reverse the order of
                // the remote and local token addresses relative to their order in the
                // finalizeBridgeERC20 function.
                _remoteToken,
                _localToken,
                _from,
                _to,
                _amount,
                _extraData
            ),
            _minGasLimit: _minGasLimit
        });
    }

tokamak-thanos/packages/tokamak/contracts-bedrock/src/universal/CrossDomainMessenger.sol_sendMessage_L185-L192

    /// @notice Sends a message to some target address on the other chain. Note that if the call
    ///         always reverts, then the message will be unrelayable, and any ETH sent will be
    ///         permanently locked. The same will occur if the target on the other chain is
    ///         considered unsafe (see the _isUnsafeTarget() function).
    /// @param _target      Target contract or wallet address.
    /// @param _message     Message to trigger the target address with.
    /// @param _minGasLimit Minimum gas limit that the message can be executed with.
    function sendMessage(address _target, bytes calldata _message, uint32 _minGasLimit) external payable {
        if (isCustomGasToken()) {
            require(msg.value == 0, "CrossDomainMessenger: cannot send value with custom gas token");
        }

        // Triggers a message to the other messenger. Note that the amount of gas provided to the
        // message is the amount of gas requested by the user PLUS the base gas value. We want to
        // guarantee the property that the call to the target contract will always have at least
        // the minimum gas limit specified by the user.
        _sendMessage({
            _to: address(otherMessenger),
            _gasLimit: baseGas(_message, _minGasLimit),
            _value: msg.value,
            _data: abi.encodeWithSelector(
                this.relayMessage.selector, messageNonce(), msg.sender, _target, msg.value, _minGasLimit, _message
            )
        });

        emit SentMessage(_target, msg.sender, _message, messageNonce(), _minGasLimit);
        emit SentMessageExtension1(msg.sender, msg.value);

        unchecked {
            ++msgNonce;
        }
    }

Impact

An attacker can bridge arbitrary tokens along with fake staking data to the StandardBridge.sol contract on the target chain. The CrossDomainMessenger.sol contract will then process the message without verifying the origin, potentially allowing malicious actors to manipulate staking incentives. This could result in significant financial loss, as tokens could be redistributed to incorrect staking targets, or manipulated in ways that drain funds from the system.

Code Snippet

tokamak-thanos/packages/tokamak/contracts-bedrock/src/universal/StandardBridge.sol_initiateBridgeETH_L331-L335

tokamak-thanos/packages/tokamak/contracts-bedrock/src/universal/StandardBridge.sol_initiateBridgeERC20_L376-L391

tokamak-thanos/packages/tokamak/contracts-bedrock/src/universal/CrossDomainMessenger.sol_sendMessage_L185-L192

Tool used

Manual Review

Recommendation

It is recommended to separate the token transfer from the message transfer, so that it is no longer necessary to call finalizeBridgeERC20 in the sendMessage function.