Lack of strict validation for gas limit because of 1 / 64 rules allow cross-chain transaction to be executed in gas limit that is less than use specify

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/bridge/Bridge.sol#L217
https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/bridge/Bridge.sol#L282
https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/bridge/Bridge.sol#L491

Vulnerability details

Lack of strict validation for gas limit because of 1 / 64 rules allow cross-chain transaction to be executed in gas limit that is less than use specify

Line of code

1. https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/bridge/Bridge.sol#L217
2. https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/bridge/Bridge.sol#L282
3. https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/bridge/Bridge.sol#L491
4. https://github.com/ethereum/EIPs/blob/master/EIPS/eip-150.md

Proof of concept

When sending cross chain from source chain to target chain,

anyone can process the message permissionlessly if the gaslimit is set to a non-zero value
Line of code

function processMessage(
        Message calldata _message,
        bytes calldata _proof
    )
        external
        nonReentrant
        whenNotPaused
        sameChain(_message.destChainId)
    {

then inside the function we call these lines of code

  // Use the specified message gas limit if called by the owner, else
  // use remaining gas
  uint256 gasLimit = msg.sender == _message.destOwner ? gasleft() : _message.gasLimit;

  if (_invokeMessageCall(_message, msgHash, gasLimit)) {
      _updateMessageStatus(msgHash, Status.DONE);
  } else {
      _updateMessageStatus(msgHash, Status.RETRIABLE);
  }

the gas limit is passed into the function _invokeMessageCall

Line of code

function _invokeMessageCall(
        Message calldata _message,
        bytes32 _msgHash,
        uint256 _gasLimit
    )
        private
        returns (bool success_)
    {
        if (_gasLimit == 0) revert B_INVALID_GAS_LIMIT();
        assert(_message.from != address(this));

        _storeContext(_msgHash, _message.from, _message.srcChainId);

        if (
            _message.data.length >= 4 // msg can be empty
                && bytes4(_message.data) != IMessageInvocable.onMessageInvocation.selector
                && _message.to.isContract()
        ) {
            success_ = false;
        } else {
            (success_,) = ExcessivelySafeCall.excessivelySafeCall(
                _message.to,
                _gasLimit,
                _message.value,
                64, // return max 64 bytes
                _message.data
            );
        }

Impact

But Because

refer to eip-150 specification

if a call asks for more gas than all but one 64th of the maximum allowed amount, 

call with all but one 64th of the maximum allowed 

amount of gas (this is equivalent to a version of EIP-90ethereum/EIPs#90 plus EIP-114ethereum/EIPs#114). 

CREATE only provides all but one 64th of the parent gas to the child call.

the 1/64 rules presences, only the 63/64 gas from gas limit is passed and spent to execute the message

then malicious user can ensure that the 63 / 64 of gasleft() is less than the message.gasLimit to make message executes but failed

The following POC shows that lack of validation on the gas limit results in failed transaction

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "forge-std/console.sol";

contract ExternalContract {
    
    function entry() public {

        try this.execute() {
            uint256 cur = gasleft();
            for (uint i = 0; i < 3650; i++) {
                i += 10;
                // console.log("i: ", i);
            }
            console.log("gas used: ", cur - gasleft());
        } catch {
            
        } 

    }

    function execute() public {
        console.log("gas left: ", gasleft());
    }

}

contract CounterTest is Test {

    using stdStorage for StdStorage;
    StdStorage stdlib;

    function setUp() public {

    }

    function testEIP150() public {
        ExternalContract externalContract = new ExternalContract();

        uint256 gasLimit = 64000;
    
        externalContract.entry{gas: gasLimit}();
    }

    function testAccurateGasLimit() public {
        ExternalContract externalContract = new ExternalContract();

        uint256 gasLimit = 100000;
    
        externalContract.entry{gas: gasLimit}();
    }



}

running 3500 for loop would cost 63513 unit of gas

but if we specify gas limit as 64000, because only 63 / 64 of gas is passed to external call, the external transaction can still marked as failed

because this allows the external transaction executes in the gas limit less than user specify

Recommendation

The recommendation is check :

gasLeft() / 63 * 64 >= message.gasLimit()

to make sure that the gas spend on execute message is at least the message.gasLimit user requires

Assessed type

ETH-Transfer

[minhquanym]

A bit similar #97 but not point out the impact

[c4-pre-sort]

minhquanym marked the issue as sufficient quality report

[dantaik]

Valid bug report, fixed in taikoxyz/taiko-mono#16613

[c4-sponsor]

dantaik (sponsor) confirmed

[adaki2004]

Agree with the proposal (confirm) but I'd disagree with severity, since gaslimit is user specified - so it could have been even fix in our UI flow, without touching the contracts.

[c4-sponsor]

adaki2004 marked the issue as disagree with severity

[c4-judge]

0xean marked the issue as duplicate of #97

[c4-judge]

0xean marked the issue as satisfactory