Livepeer contest - gzeon's results

Handle

gzeon

Vulnerability details

Impact

migrateETH does not send the withdrawn ETH to L2 causing fund to stuck in the L1Migrator contract.

Proof of Concept

When migrateETH is called, it would withdraw all ETH from bridgeMinter, and then use sendTxToL2 create a L2 retryable ticket to call the L2Migrator with _l1CallValue set to msg.value and _l2CallValue set to the ETH balance withdrew.

https://github.com/livepeer/protocol/blob/20e7ebb86cdb4fe9285bf5fea02eb603e5d48805/contracts/token/BridgeMinter.sol#L90

    function withdrawETHToL1Migrator() external onlyL1Migrator returns (uint256) {
        uint256 balance = address(this).balance;

        // call() should be safe from re-entrancy here because the L1Migrator and l1MigratorAddr are trusted
        (bool ok, ) = l1MigratorAddr.call.value(address(this).balance)("");
        require(ok, "BridgeMinter#withdrawETHToL1Migrator: FAIL_CALL");

        return balance;
    }

https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L1/gateway/L1Migrator.sol#L303

    function migrateETH(
        uint256 _maxGas,
        uint256 _gasPriceBid,
        uint256 _maxSubmissionCost
    ) external payable whenNotPaused {
        uint256 amount = IBridgeMinter(bridgeMinterAddr)
            .withdrawETHToL1Migrator();

        // Any ETH refunded to the L2 alias of this contract can be used for
        // other cross-chain txs sent by this contract.
        // The retryable ticket created will not be cancellable since this contract
        // currently does not support cross-chain txs to call ArbRetryableTx.cancel().
        // Regarding the comment below on this contract receiving refunds:
        // msg.sender also cannot be the address to receive refunds as beneficiary because otherwise
        // msg.sender could cancel the ticket before it is executed on L2 to receive the L2 call value.
        sendTxToL2(
            l2MigratorAddr,
            address(this), // L2 alias of this contract will receive refunds
            msg.value,
            amount,
            _maxSubmissionCost,
            _maxGas,
            _gasPriceBid,
            ""
        );
    }

The sendTxToL2 function create a L2 retryable ticket using _l1CallValue as call value. The ETH in _l2CallValue is not transfered to the inbox contract.

https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L1/gateway/L1ArbitrumMessenger.sol#L55

    function sendTxToL2(
        address target,
        address from,
        uint256 _l1CallValue,
        uint256 _l2CallValue,
        uint256 maxSubmissionCost,
        uint256 maxGas,
        uint256 gasPriceBid,
        bytes memory data
    ) internal returns (uint256) {
        uint256 seqNum = inbox.createRetryableTicket{value: _l1CallValue}(
            target,
            _l2CallValue,
            maxSubmissionCost,
            from,
            from,
            maxGas,
            gasPriceBid,
            data
        );
        emit TxToL2(from, target, seqNum, data);
        return seqNum;
    }

Since the _l2CallValue (i.e. the ETH withdrew from bridgeMinter) was never sent to the inbox, it would stuck in the L1 migrator contract and the L2 call would likely revert due to insufficient call value.

Recommended Mitigation Steps

The call value should be _l1CallValue + _l2CallValue

Handle

gzeon

Vulnerability details

Impact

Anyone can call migrateETH and migrateLPT in L1Migrator with arbitrary _maxSubmissionCost. For example when migrateLPT is called, it would withdraw all LPT from bridgeMinter, and then create a L2 retryable ticket to call the L2Migrator using _maxSubmissionCost supplied by the caller.

    function migrateLPT(
        uint256 _maxGas,
        uint256 _gasPriceBid,
        uint256 _maxSubmissionCost
    ) external payable whenNotPaused {
        uint256 amount = IBridgeMinter(bridgeMinterAddr)
            .withdrawLPTToL1Migrator();

        // Approve L1LPTGateway to pull tokens
        ApproveLike(tokenAddr).approve(l1LPTGatewayAddr, amount);
        // Trigger cross-chain transfer with L1LPTGateway which will pull and escrow tokens
        // Forward msg.value to outboundTransfer() to be used for cross-chain tx
        IL1LPTGateway(l1LPTGatewayAddr).outboundTransfer{value: msg.value}(
            tokenAddr,
            l2MigratorAddr,
            amount,
            _maxGas,
            _gasPriceBid,
            abi.encode(_maxSubmissionCost, "")
        );
    }

If the caller supplied a very low (e.g. 0) _maxSubmissionCost, the call would succeed on L1, but would fail the retryable ticket creation on L2.

https://developer.offchainlabs.com/docs/l1_l2_messages#important-note-about-base-submission-fee

If an L1 transaction underpays for a retryable ticket's base submission free, the retryable ticket creation on L2 simply fails. Given that this potentially breaks the atomicity of the L1 / L2 transactions, applications should avoid this scenario.

Since both migrateETH and migrateLPT are permissionless, an attacker can freeze the fund by calling the functions with _maxSubmissionCost=0.

Recommended Mitigation Steps

Use ArbRetryableTx.getSubmissionPrice provided by Arbitrum L1 contract instead of using user provided _maxSubmissionCost.

Handle

gzeon

Vulnerability details

Impact

L1Migrator can call withdrawETHToL1Migrator to withdraw ETH from BridgeMinter, but L1Migrator does not have a payable receive function so the call will revert.

Proof of Concept

https://github.com/livepeer/protocol/blob/20e7ebb86cdb4fe9285bf5fea02eb603e5d48805/contracts/token/BridgeMinter.sol#L90

    function withdrawETHToL1Migrator() external onlyL1Migrator returns (uint256) {
        uint256 balance = address(this).balance;

        // call() should be safe from re-entrancy here because the L1Migrator and l1MigratorAddr are trusted
        (bool ok, ) = l1MigratorAddr.call.value(address(this).balance)("");
        require(ok, "BridgeMinter#withdrawETHToL1Migrator: FAIL_CALL");

        return balance;
    }

Recommended Mitigation Steps

Add a payable receive function

Handle

gzeon

Vulnerability details

Impact

Fund can be lost if the L1 call value provided is insufficient to cover _maxSubmissionCost, or stuck if insufficient to cover _maxSubmissionCost + (_maxGas * _gasPriceBid).

Proof of Concept

outboundTransfer in L1LPTGateway does not check if the call value is sufficient, if it is < _maxSubmissionCost the retryable ticket creation will fail and fund is lost; if it is <_maxSubmissionCost + (_maxGas * _gasPriceBid) the ticket would require manual execution.

https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L1/gateway/L1LPTGateway.sol#L80

    function outboundTransfer(
        address _l1Token,
        address _to,
        uint256 _amount,
        uint256 _maxGas,
        uint256 _gasPriceBid,
        bytes calldata _data
    ) external payable override whenNotPaused returns (bytes memory) {
        require(_l1Token == l1Lpt, "TOKEN_NOT_LPT");

        // nested scope to avoid stack too deep errors
        address from;
        uint256 seqNum;
        bytes memory extraData;
        {
            uint256 maxSubmissionCost;
            (from, maxSubmissionCost, extraData) = parseOutboundData(_data);
            require(extraData.length == 0, "CALL_HOOK_DATA_NOT_ALLOWED");

            // transfer tokens to escrow
            TokenLike(_l1Token).transferFrom(from, l1LPTEscrow, _amount);

            bytes memory outboundCalldata = getOutboundCalldata(
                _l1Token,
                from,
                _to,
                _amount,
                extraData
            );

            seqNum = sendTxToL2(
                l2Counterpart,
                from,
                maxSubmissionCost,
                _maxGas,
                _gasPriceBid,
                outboundCalldata
            );
        }

        emit DepositInitiated(_l1Token, from, _to, seqNum, _amount);

        return abi.encode(seqNum);
    }

Recommended Mitigation Steps

Add check similar to the one used in L1GatewayRouter provided by Arbitrum team

https://github.com/OffchainLabs/arbitrum/blob/b8366005a697000dda1f57a78a7bdb2313db8fe2/packages/arb-bridge-peripherals/contracts/tokenbridge/ethereum/gateway/L1GatewayRouter.sol#L236

        uint256 expectedEth = _maxSubmissionCost + (_maxGas * _gasPriceBid);
        require(_maxSubmissionCost > 0, "NO_SUBMISSION_COST");
        require(msg.value == expectedEth, "WRONG_ETH_VALUE");