sherlock-audit / 2024-08-tokamak-network-judging

1 stars 0 forks source link

GGONE - Gas Estimation and Insufficient Buffer in CrossDomainMessenger May Lead to Fund Loss #36

Open sherlock-admin4 opened 3 weeks ago

sherlock-admin4 commented 3 weeks ago

GGONE

Medium

Gas Estimation and Insufficient Buffer in CrossDomainMessenger May Lead to Fund Loss

Summary

Reference Link

https://solodit.xyz/issues/cross-domain-messengers-can-fail-in-relaying-a-message-openzeppelin-none-mantle-v2-solidity-contracts-audit-markdown

This report identifies a vulnerability in the CrossDomainMessenger related to gas estimation and buffer sizes during external calls. The insufficient gas buffer can lead to transaction failures, potentially resulting in the loss of user funds.

Vulnerability Detail

One characteristic of the original relayMessage function is its gas estimation process, which includes several operations before the external call. https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/packages/contracts-bedrock/src/universal/CrossDomainMessenger.sol#L267-L287

        if (
            !SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER)
                || xDomainMsgSender != Constants.DEFAULT_L2_SENDER
        ) {
            failedMessages[versionedHash] = true;
            emit FailedRelayedMessage(versionedHash);

            // Revert in this case if the transaction was triggered by the estimation address. This
            // should only be possible during gas estimation or we have bigger problems. Reverting
            // here will make the behavior of gas estimation change such that the gas limit
            // computed will be the amount required to relay the message, even if that amount is
            // greater than the minimum gas limit specified by the user.
            if (tx.origin == Constants.ESTIMATION_ADDRESS) {
                revert("CrossDomainMessenger: failed to relay message");
            }

            return;
        }

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

The hasMinGas function performs the gas check and has clear documentation that warns about the overhead gas. It states that 40,000 units of gas are added to account for worst-case scenarios when executing the CALL opcode in subsequent external calls.

For instance, under the Shanghai EVM version, worst-case scenarios include:

Access to cold storage requires 2600 gas units. Calling a non-existent target requires 25000 gas units. A positive msg.value in the call increases the cost by 9000 gas units.

Also, note that the second argument of the hasMinGas function is the sum of the following two variables:

RELAY_RESERVED_GASwhich is set to 40000 units of gas. This is an estimation of how much gas is needed to continue with the relayMessage execution after the external call. This is unchanged from Optimism code. RELAY_GAS_CHECK_BUFFERwhich is set to 5000 units of gas and represents an amount that should be used in between the hasMinGas function and the external call. This is also unchanged from Optimism code.

The hasMinGas function uses the following formula:

[ gasLeft - (40000 + _reservedGas) ] * 63/64 >= _minGas

Here, _reservedGas is 45,000 units, with only 5000 estimated as a buffer before the external call. Thus, the total buffer may not be sufficient, especially when relaying messages that include ERC20 token approvals, which can exceed the provided gas buffer.

Assume a scenario where the relayed message includes an approval operation for an ERC-20 token. The gas consumption for such an approval can range from a few thousand to 30,000 or 40,000 units of gas, far exceeding the few thousand units provided by the current buffer. Some instances of an OptmismMintableERC20 token might even consume more than 40,000 units of gas per approval call. The gas consumption for an approve call depends on whether the values are being set from zero to positive, vice versa, or from non-zero to non-zero values. A similar concern applies to the relayMessage function of the L2CrossDomainMessenger contract, where inadequate gas buffers can lead to transaction failures.

Impact

The insufficient gas buffer may result in transaction failures, leading to irrecoverable fund loss for users. If a transaction fails due to gas issues, users cannot replay the transaction, which opens avenues for potential denial-of-service (DoS) attacks.

Code Snippet

Tool used

Manual Review

Recommendation

Reference Link

https://github.com/mantlenetworkio/mantle-v2/commit/92ebaf96622e8126ce5322bdf3ea730640c7a548

Revisit the values of RELAY_GAS_CHECK_BUFFER and RELAY_RESERVED_GAS to ensure that the gas estimates are sufficient to handle worst-case scenarios. Adjust the gas buffers to avoid unexpected gas failures and ensure transactions can complete successfully. Additionally, implement logic to allow calls to relayMessage to fail gracefully, preventing issues with deposits and withdrawals.