ks__xxxxx - SafeCall.call() function is still being used even after having SafeCall.callWithMinGas() which follows the EIP-150 63/64th Rule for Gas

[sherlock-admin]

ks__xxxxx

medium

SafeCall.call() function is still being used even after having SafeCall.callWithMinGas() which follows the EIP-150 63/64th Rule for Gas

Summary

SafeCall.call() function is being used without following the follows the EIP-150 63/64th Rule for Gas which could result in the locking of funds permenantly.

I am filing the finding as Medium Severity as we are still using legacy function instead of a an updated function which follows EIP 150 63/64th Rule for Gas guidelines perfectly

Vulnerability Detail

finalizeBridgeETH() function in StandardBridge.sol is using the legacy SafeCall.call() function instead of using the latest SafeCall.callWithMinGas() which follows the EIP-150 63/64th Rule for Gas.

As we know that the EVM limits the total gas forwarded on to 63/64ths of the total gasleft(), so if a malicious user call this finalizeBridgeETH() function with a limited amount of gas ( when gasLeft() < minimum amount of gas required for transaction ), the transactions will fail and also since there are no replays in the contract the user funds will be locked.

Impact

A malicious user can call finalizeBridgeERC20() when the gasLeft() is less than minimum gas required to pass the call, It would result in their funds permanently locked in the OptimismPortal contract.

Tagging this as medium

Code Snippet

https://github.com/sherlock-audit/2023-03-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/L1/L1StandardBridge.sol#L222
https://github.com/sherlock-audit/2023-03-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/L2/L2StandardBridge.sol#L152
https://github.com/sherlock-audit/2023-03-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/universal/StandardBridge.sol#L308

Tool used

Manual Review

Recommendation

use SafeCall.callWithMinGas() function instead of SafeCall.call() which does an assertion on gasLeft() using the 63/64th Rule for Gas

[GalloDaSballo]

Seems to be dup of #37 which is Dup of #40 although has a loss of detail compared to those

[Jiaren-tang]

This is not valid because the require statement makes sure after the Safe.call, the underlying call cannot revert.

    function finalizeBridgeETH(
        address _from,
        address _to,
        uint256 _amount,
        bytes calldata _extraData
    ) public payable onlyOtherBridge {
        require(msg.value == _amount, "StandardBridge: amount sent does not match amount required");
        require(_to != address(this), "StandardBridge: cannot send to self");
        require(_to != address(MESSENGER), "StandardBridge: cannot send to messenger");

        // Emit the correct events. By default this will be _amount, but child
        // contracts may override this function in order to emit legacy events as well.
        _emitETHBridgeFinalized(_from, _to, _amount, _extraData);

        bool success = SafeCall.call(_to, gasleft(), _amount, hex"");
        require(success, "StandardBridge: ETH transfer failed");
    }

If a malicious attacker wants to take the advantage of the EIP 150 and make the call silently revert,

bool success = SafeCall.call(_to, gasleft(), _amount, hex"");

the require statement would catch that and revert the transaction.

require(success, "StandardBridge: ETH transfer failed");

[Jiaren-tang]

The report also lacks prove and POC and lacks details.

[hrishibhat]

Sponsor comment:
This should not be the case since the call will revert if out of gas in the same way callWithMinGas does. Having gasleft() not satisfy the gas required for the call is user error and is the intended behavior.

[GalloDaSballo]

Siding with sponsor, as User Mistake