IzuMan - `Paused Bridge` will allow `Users` to Initiate a bridge and `Lock Funds` In the Bridge

[sherlock-admin3]

IzuMan

Medium

Paused Bridge will allow Users to Initiate a bridge and Lock Funds In the Bridge

Summary

There are no checks for if the bridge is paused when initiating a bridging event, this can lead to loss of user funds since the transaction will be successful one side but always revert when the transaction is attempted to be finalized on the other side. This can lead to a loss of user funds that will be locked in the bridge.

Root Cause

No checks for the paused status:

• One example is found here: GitHub Link

    function depositETH(uint32 _minGasLimit, bytes calldata _extraData) external payable onlyEOA {
      _initiateETHDeposit(msg.sender, msg.sender, _minGasLimit, _extraData);
  }

• Example here: GitHub Link
• There are additional entry points in the contract.

Internal pre-conditions

This test first sets the bridge state to paused by mocking the guardian address and calling the SuperChainConfig::pause. Even though the bridge is paused anyone can freely call the functions inside the bridge contract to initiate a bridging event. However, the finalizing part of the the bridging event does check the paused state of the contract and will always revert. As a result, user will still deposit into the bridge and lose their funds since the transaction will never be finalized on the layer 1 chain or the layer 2 chain respectively.

External pre-conditions

No response

Attack Path

No response

Impact

Users can initiate a bridge of tokens/ether that will never be finalized and locked in the contract if the bridge is paused since the messages cannot be replayed.

PoC

Place the following contract into L1StandardBridge.t.sol and the run the test with the following command forge test --mt test_paused_bridge_can_still_initiate_bridging

contract Audit_Tests is Bridge_Initializer {
    ////////////////////////////////
    //         AUDIT ADDED        //
    ////////////////////////////////

    function test_paused_bridge_can_still_initiate_bridging() external {
        vm.prank(superchainConfig.guardian());
        superchainConfig.pause("identifier");

        uint256 nonce = l1CrossDomainMessenger.messageNonce();
        uint256 version = 0; // Internal constant in the OptimismPortal: DEPOSIT_VERSION
        address l1MessengerAliased = AddressAliasHelper.applyL1ToL2Alias(address(l1CrossDomainMessenger));

        bytes memory message = abi.encodeWithSelector(
            StandardBridge.finalizeBridgeERC20.selector, address(L2Token), address(L1Token), alice, bob, 1000, hex""
        );

        bytes memory innerMessage = abi.encodeWithSelector(
            CrossDomainMessenger.relayMessage.selector,
            nonce,
            address(l1StandardBridge),
            address(l2StandardBridge),
            0,
            10000,
            message
        );

        uint64 baseGas = l1CrossDomainMessenger.baseGas(message, 10000);
        bytes memory opaqueData = abi.encodePacked(uint256(0), uint256(0), baseGas, false, innerMessage);

        deal(address(L1Token), alice, 100000, true);

        vm.prank(alice);
        L1Token.approve(address(l1StandardBridge), type(uint256).max);

        // Should emit both the bedrock and legacy events
        vm.expectEmit(address(l1StandardBridge));
        emit ERC20DepositInitiated(address(L1Token), address(L2Token), alice, bob, 1000, hex"");

        vm.expectEmit(address(l1StandardBridge));
        emit ERC20BridgeInitiated(address(L1Token), address(L2Token), alice, bob, 1000, hex"");

        // OptimismPortal emits a TransactionDeposited event on `depositTransaction` call
        vm.expectEmit(address(optimismPortal));
        emit TransactionDeposited(l1MessengerAliased, address(l2CrossDomainMessenger), version, opaqueData);

        // SentMessage event emitted by the CrossDomainMessenger
        vm.expectEmit(address(l1CrossDomainMessenger));
        emit SentMessage(address(l2StandardBridge), address(l1StandardBridge), message, nonce, 10000);

        // SentMessageExtension1 event emitted by the CrossDomainMessenger
        vm.expectEmit(address(l1CrossDomainMessenger));
        emit SentMessageExtension1(address(l1StandardBridge), 0);

        // the L1 bridge should call L1CrossDomainMessenger.sendMessage
        vm.expectCall(
            address(l1CrossDomainMessenger),
            abi.encodeWithSelector(CrossDomainMessenger.sendMessage.selector, address(l2StandardBridge), message, 10000)
        );
        // The L1 XDM should call OptimismPortal.depositTransaction
        vm.expectCall(
            address(optimismPortal),
            abi.encodeWithSelector(
                OptimismPortal.depositTransaction.selector,
                address(l2CrossDomainMessenger),
                0,
                baseGas,
                false,
                innerMessage
            )
        );
        vm.expectCall(
            address(L1Token),
            abi.encodeWithSelector(ERC20.transferFrom.selector, alice, address(l1StandardBridge), 1000)
        );

        vm.prank(alice);
        l1StandardBridge.depositERC20To(address(L1Token), address(L2Token), bob, 1000, 10000, hex"");

        assertEq(l1StandardBridge.deposits(address(L1Token), address(L2Token)), 1000);
    }
}

Mitigation

Include the whenNotPaused modifier on functions that initiate a bridging event.