ChainPatrol - Gas usage of cross-chain messages is undercounted, causing discrepancy between L1 and L2 and impacting intrinsic gas calculation

[sherlock-admin2]

ChainPatrol

Medium

Gas usage of cross-chain messages is undercounted, causing discrepancy between L1 and L2 and impacting intrinsic gas calculation

Summary

Gas consumption of messages sent via CrossDomainMesenger (including both L1CrossDomainMesenger and L2CrossDomainMesenger is calculated incorrectly: the gas usage of the relayMessage wrapper is not counted. As a result, the actual gas consumption of sending a message will be higher than expected. Users will pay less for gas on L1, and L2 blocks may be filled earlier than expected.

Vulnerability Detail

The CrossDomaingMesenger::sendMessage function is used to send cross-chain messages. Users are required to set the _minGasLimit argument, which is the expected amount of gas that the message will consume on the other chain. This is done in the baseGas function which computes the byte-wise cost of the message. CrossDomainMessenger also allows users to replay their messages on the destination chain if they failed: to allow this, the contract wraps user messages in relayMessage calls. This increases the size of messages, but the baseGas call above counts gas usage of only the original, not wrapped in the relayMessage call, message.

This behaviour also disagrees with how the migration process works:

• when migrating pre-Bedrock withdrawals, data is the entire messages, including the relayMessage calldata.
• the gas limit of migrated messages is computed on the entire data.

Taking into account the logic of paying cross-chain messages gas consumption on L1, I think the implementation in the migration code is correct and the implementation in CrossDomainMessenger is wrong: users should pay for sending the entire cross-chain message, not just the calldata that will be execute on the recipient on the other chain.

Impact

Since the CrossDomainMessenger contract is recommended to be used as the main cross-chain messaging contract and since it's used by both L1 and L2 bridges (when bridging ERC20 tokens), the undercounted gas will have a broad impact on the system. It'll create a discrepancy in gas usage and payment on L1 and L2: on L1, users will pay for less gas than actually will be consumed by cross-chain messages.

The following bytes are excluded from gas usage counting: link

• the 4 bytes of the relayMessage selector;
• the 32 bytes of the message nonce;
• the address of the sender;
• the address of the recipient;
• the amount of ETH sent with the message;
• the minimal gas limit of the nested message;
  Thus, every cross-chain message sent via the bridge or the messenger will contain 140 bytes that won't be paid by users. The bytes will however be processed by the node and accounted in the gas consumption.

Code Snippet

• CrossDomainMessenger.sendMessage sends cross-chain messages: https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/packages/tokamak/contracts-bedrock/src/universal/CrossDomainMessenger.sol#L176

• CrossDomainMessenger.sendMessage wraps cross-chain messages in relayMessage calls: https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/packages/tokamak/contracts-bedrock/src/universal/CrossDomainMessenger.sol#L185-L187

• The gas limit counting of cross-chain messages includes only the length of the nested message and doesn't include the relayMessage wrapping: https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/packages/tokamak/contracts-bedrock/src/universal/CrossDomainMessenger.sol#L183

• When pre-Bedrock withdrawals are migrated, gas limit calculation does include the relayMessage wrapping: https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/op-chain-ops/crossdomain/migrate.go#L49-L62

Tool used

Manual Review

Recommendation

When counting gas limit in the CrossDomainMessenger.sendMessage function, consider counting the entire message, including the relayMessage calldata wrapping. Consider a change like that:

function sendMessage(address _target, bytes calldata _message, uint32 _minGasLimit) external payable {
        if (isCustomGasToken()) {
            require(msg.value == 0, "CrossDomainMessenger: cannot send value with custom gas token");
        }

        // Triggers a message to the other messenger. Note that the amount of gas provided to the
        // message is the amount of gas requested by the user PLUS the base gas value. We want to
        // guarantee the property that the call to the target contract will always have at least
        // the minimum gas limit specified by the user.

          bytes memory wrappedMessage = abi.encodeWithSelector(
              this.relayMessage.selector,
              messageNonce(),
              msg.sender,
              _target,
              msg.value,
             _minGasLimit,
             _message
        );

        _sendMessage({
            _to: address(otherMessenger),
            _gasLimit: baseGas(wrappedMessage , _minGasLimit),
            _value: msg.value,
            wrappedMessage 
        });

        emit SentMessage(_target, msg.sender, _message, messageNonce(), _minGasLimit);
        emit SentMessageExtension1(msg.sender, msg.value);

        unchecked {
            ++msgNonce;
        }
    }