unforgiven - [High] funds in FeeVault and child contracts in L2 can be locked because withdraw() specify hardcoded and low amount of gas when calling bridgeETHTo()

[github-actions[bot]]

unforgiven

high

[High] funds in FeeVault and child contracts in L2 can be locked because withdraw() specify hardcoded and low amount of gas when calling bridgeETHTo()

Summary

Contract FeeVault contains the basic logic for the various different vault contracts used to hold fee revenue generated by the L2 system. Contracts BaseFeeVault, SequencerFeeVault and L1FeeVault are extending FeeVault. Function withdraw() in FeeVault sends funds to recipient in the L1 with the help of the L2StandardBridge contract's bridgeETHTo() functions. the specified gas is when calling bridgeETHTo() is 20000 which is less that required amount for calling a contract or sending a eth. this can cause funds to be looked in the bridge and not received by the recipient.

Vulnerability Detail

This is withdraw() code:

    /**
     * @notice Triggers a withdrawal of funds to the L1 fee wallet.
     */
    function withdraw() external {
        require(
            address(this).balance >= MIN_WITHDRAWAL_AMOUNT,
            "FeeVault: withdrawal amount must be greater than minimum withdrawal amount"
        );

        uint256 value = address(this).balance;
        totalProcessed += value;

        emit Withdrawal(value, RECIPIENT, msg.sender);

        L2StandardBridge(payable(Predeploys.L2_STANDARD_BRIDGE)).bridgeETHTo{ value: value }(
            RECIPIENT,
            20000,
            bytes("")
        );
    }

As you can see the specified gas is hard coded and it's 20000 which is not enough for fund transfer. also the gas usage can change in the ETH and it can cause more serious problem. so using hardcoded gas amount is not recommended and can cause vault's funds to be locked in the bridge.

Impact

This can cause fund loss for the vaults as they funds would stuck in the bridge and in the best case scenario multiple transaction need to be made in the L1 to withdraw the funds and extra gas would be spend.

Code Snippet

https://github.com/ethereum-optimism/optimism/blob/3f4b3c328153a8aa03611158b6984d624b17c1d9/packages/contracts-bedrock/contracts/universal/FeeVault.sol#L65-L69

Tool used

Manual Review

Recommendation

specify more gas to make sure transaction won't revert and also give admin the ability to increase the gas between MIN and MAX value.

[rcstanciu]

Comment from Optimism

---

Description: Funds in FeeVault and child contracts in L2 can be locked because withdraw() specify hardcoded and low amount of gas when calling bridgeETHTo()

Reason: The reporter is slightly correct, but mostly incorrect. The amount of gas supplied for the withdrawal transaction is not hard-coded, 20000 is the minimum gas limit required to execute the withdrawal. However, the reporter is correct that a 20000 minimum is too low (the cost to transfer ETH is 21000.)

Action: Raise the minimum gas limit in the FeeVault's withdraw function so that the withdrawal of fees cannot be bricked by supplying a gas limit that is too low when finalizing on L1.