079.md

csanuragjain

high

Steal all funds

Summary

By carefully crafting the _data while calling initiateWithdrawal, attacker can steal all user funds

Vulnerability Detail

1. Attacker calls initiateWithdrawal method with below params:

_target = L1 CrossDomainMessenger

// relayMessage(_nonce, _sender, _target, _value, _minGasLimit, _message)
_data = relayMessage(messageNonce(), L2StandardBridge, L1StandardBridge, _value, _minGasLimit, finalizeBridgeERC20(WETH,WethWrappedRemote,L1StandardBridgeORVictimUser,Attacker,X,""))

2. This withdraw is finalized and executed by Portal

3. relayMessage is called on L1 CrossDomainMessenger contract with below args

_nonce = messageNonce()
_sender = L2StandardBridge
_target = L1StandardBridge
_value= _value
_minGasLimit= _minGasLimit
_message= finalizeBridgeERC20(WETH,WethWrappedRemote,L1StandardBridgeORVictimUser,Attacker,X,""))

4. Below code then call the target from args with calldata in step 3

xDomainMsgSender = _sender;
bool success = SafeCall.call(_target, gasleft() - RELAY_GAS_BUFFER, _value, _message);

5. xDomainMsgSender is set to L2StandardBridge

6. finalizeBridgeERC20(WETH,WethWrappedRemote,L1StandardBridgeORVictimUser,Attacker,X,"")) is called on L1StandardBridge contract

function finalizeBridgeERC20(
        address _localToken,
        address _remoteToken,
        address _from,
        address _to,
        uint256 _amount,
        bytes calldata _extraData
    ) public onlyOtherBridge {
        if (_isOptimismMintableERC20(_localToken)) {
            require(
                _isCorrectTokenPair(_localToken, _remoteToken),
                "StandardBridge: wrong remote token for Optimism Mintable ERC20 local token"
            );

            OptimismMintableERC20(_localToken).mint(_to, _amount);
        } else {
            deposits[_localToken][_remoteToken] = deposits[_localToken][_remoteToken] - _amount;
            IERC20(_localToken).safeTransfer(_to, _amount);
        }

        emit ERC20BridgeFinalized(_localToken, _remoteToken, _from, _to, _amount, _extraData);
    }

7. Since xDomainMsgSender is L2StandardBridge so onlyOtherBridge pass

8. Now function executes and below code executes

deposits[_localToken][_remoteToken] = deposits[_localToken][_remoteToken] - _amount;
            IERC20(_localToken).safeTransfer(_to, _amount);

9. Since _localToken is WETH and _remoteToken is WethWrappedRemote so if current deposit for WETH is decreased by amount X and the same amount X is transferred to Attacker

Impact

Attacker can steal all bridge funds

Code Snippet

https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/L2/L2ToL1MessagePasser.sol#L98

Tool used

Manual Review

Recommendation

initiateWithdrawal function should only be allowed to be called via L2 CrossDomainMessenger