importDev0x - Smart contract users can Bridge ETH or ERC20 tokens from their account to themselves with `bridgeETHTo` and `bridgeERC20To` functions

[sherlock-admin4]

importDev0x

High

Smart contract users can Bridge ETH or ERC20 tokens from their account to themselves with bridgeETHTo and bridgeERC20To functions

Summary

Smart contract users are limited to bridging ETH or ERC20 tokens to themselves because of the onlyEOA modifier in the bridgeETH and bridgeERC20 functions. However, they can bridge ETH or ERC20 tokens using the bridgeETHTo and bridgeERC20To functions without any restrictions, which provide the same functionality as bridgeETH and bridgeERC20. The only requirement for smart contract users is to set the _to parameter of the bridgeETHTo and bridgeERC20To functions to the address of their contract.

https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/packages/contracts-bedrock/src/universal/StandardBridge.sol#L187

https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/packages/contracts-bedrock/src/universal/StandardBridge.sol#L222

Vulnerability Detail

In the test below, it is expected that a smart contract user cannot bridge ETH when the destination address (_to) is set to msg.sender. However, it runs without any issues, resulting in the test failing.

Please copy the test function below and add it to the end of the L1StandardBridge.t.sol file.

  contract MyL1StandardBridge is Bridge_Initializer {
    function test_smartContractUserCanBridgeETH() external {
      vm.startPrank(bob);
      UserContract userContract = new UserContract(l1StandardBridge);
      (bool success, ) = address(userContract).call{value: 1 ether}('');
      require(success, 'failed');
      vm.expectRevert();
      userContract.bridgeWith_bridgeETHTo();
      vm.stopPrank();
    }
  }

  contract UserContract {
    L1StandardBridge l1StandardBridge;
    address owner;

    constructor(L1StandardBridge _l1Bridge) {
      l1StandardBridge = _l1Bridge;
      owner = msg.sender;
    }

    modifier onlyOwner() {
      if (msg.sender != owner) revert();
      _;
    }

    function bridgeWith_bridgeETH() external onlyOwner {
      l1StandardBridge.bridgeETH{value: 1 ether}(50000, hex'');
    }

    function bridgeWith_bridgeETHTo() external onlyOwner {
      l1StandardBridge.bridgeETHTo{value: 1 ether}(msg.sender, 50000, hex'');
    }

    receive() external payable {}
}

Recommendation

msg.sender and _to should not be the same in the bridgeETHTo and bridgeERC20To functions. Therefore, we can perform this check at the beginning of these two functions. If EOA (Externally Owned Account) users want to bridge ETH or ERC20 tokens to themselves, they can use the bridgeETH and bridgeERC20 functions as intended.

function bridgeETHTo(address _to, uint32 _minGasLimit, bytes calldata _extraData) public payable {
+        require(_to != msg.sender, "use bridgeETH instead!!");
        _initiateBridgeETH(msg.sender, _to, msg.value, _minGasLimit, _extraData);
    }

    function bridgeERC20To(
        address _localToken,
        address _remoteToken,
        address _to,
        uint256 _amount,
        uint32 _minGasLimit,
        bytes calldata _extraData
    )
        public
        virtual
    {
+        require(_to != msg.sender, "use bridgeERC20 instead!!");
        _initiateBridgeERC20(_localToken, _remoteToken, msg.sender, _to, _amount, _minGasLimit, _extraData);
    }