feat(protocol): improve Bridge gasleft() validation

packages/protocol/contracts/bridge/Bridge.sol

@@ -25,6 +25,7 @@ contract Bridge is EssentialContract, IBridge {
         uint32 gasUsedInFeeCalc;
         uint32 proofSize;
         uint32 numCacheOps;
+        bool processedByRelayer;
     }
 
     /// @dev A debug event for fine-tuning gas related constants in the future.
@@ -39,6 +40,9 @@ contract Bridge is EssentialContract, IBridge {
     /// This value should be fine-tuned with production data.
     uint32 public constant GAS_OVERHEAD = 120_000;
 
+    ///@dev The max proof size for a message to be processable by a relayer.
+    uint256 public constant RELAYER_MAX_PROOF_BYTES = 200_000;
+
     /// @dev The amount of gas not to charge fee per cache operation.
     uint256 private constant _GAS_REFUND_PER_CACHE_OPERATION = 20_000;
 
@@ -87,6 +91,7 @@ contract Bridge is EssentialContract, IBridge {
     error B_MESSAGE_NOT_SENT();
     error B_OUT_OF_ETH_QUOTA();
     error B_PERMISSION_DENIED();
+    error B_PROOF_TOO_LARGE();
     error B_RETRY_FAILED();
     error B_SIGNAL_NOT_RECEIVED();
 
@@ -134,7 +139,7 @@ contract Bridge is EssentialContract, IBridge {
     {
         if (_message.gasLimit == 0) {
             if (_message.fee != 0) revert B_INVALID_FEE();
-        } else if (_invocationGasLimit(_message, false) == 0) {
+        } else if (_invocationGasLimit(_message) == 0) {
             revert B_INVALID_GAS_LIMIT();
         }
 
@@ -227,17 +232,20 @@ contract Bridge is EssentialContract, IBridge {
             revert B_INVALID_CHAINID();
         }
 
+        ProcessingStats memory stats;
+        stats.processedByRelayer = msg.sender != _message.destOwner;
+
         // If the gas limit is set to zero, only the owner can process the message.
-        if (_message.gasLimit == 0 && msg.sender != _message.destOwner) {
-            revert B_PERMISSION_DENIED();
+        if (stats.processedByRelayer) {
+            if (_message.gasLimit == 0) revert B_PERMISSION_DENIED();
+            if (_proof.length > RELAYER_MAX_PROOF_BYTES) revert B_PROOF_TOO_LARGE();
         }
 
         bytes32 msgHash = hashMessage(_message);
         _checkStatus(msgHash, Status.NEW);
 
         address signalService = resolve(LibStrings.B_SIGNAL_SERVICE, false);
 
-        ProcessingStats memory stats;
         stats.proofSize = uint32(_proof.length);
         stats.numCacheOps =
             _proveSignalReceived(signalService, msgHash, _message.srcChainId, _proof);
@@ -252,11 +260,9 @@ contract Bridge is EssentialContract, IBridge {
             status_ = Status.DONE;
             reason_ = StatusReason.INVOCATION_PROHIBITED;
         } else {
-            uint256 gasLimit = msg.sender == _message.destOwner
-                ? gasleft() // ignore _message.gasLimit
-                : _invocationGasLimit(_message, true);
+            uint256 gasLimit = stats.processedByRelayer ? _invocationGasLimit(_message) : gasleft();
 
-            if (_invokeMessageCall(_message, msgHash, gasLimit)) {
+            if (_invokeMessageCall(_message, msgHash, gasLimit, stats.processedByRelayer)) {
                 status_ = Status.DONE;
                 reason_ = StatusReason.INVOCATION_OK;
             } else {
@@ -268,7 +274,7 @@ contract Bridge is EssentialContract, IBridge {
         if (_message.fee != 0) {
             refundAmount += _message.fee;
 
-            if (msg.sender != _message.destOwner && _message.gasLimit != 0) {
+            if (stats.processedByRelayer && _message.gasLimit != 0) {
                 unchecked {
                     uint256 refund = stats.numCacheOps * _GAS_REFUND_PER_CACHE_OPERATION;
                     stats.gasUsedInFeeCalc = uint32(GAS_OVERHEAD + gasStart - gasleft());
@@ -313,14 +319,14 @@ contract Bridge is EssentialContract, IBridge {
             uint256 invocationGasLimit;
             if (msg.sender != _message.destOwner) {
                 if (_message.gasLimit == 0 || _isLastAttempt) revert B_PERMISSION_DENIED();
-                invocationGasLimit = _invocationGasLimit(_message, true);
+                invocationGasLimit = _invocationGasLimit(_message);
             } else {
                 // The owner uses all gas left in message invocation
                 invocationGasLimit = gasleft();
             }
 
             // Attempt to invoke the messageCall.
-            succeeded = _invokeMessageCall(_message, msgHash, invocationGasLimit);
+            succeeded = _invokeMessageCall(_message, msgHash, invocationGasLimit, false);
         }
 
         if (succeeded) {
@@ -477,13 +483,15 @@ contract Bridge is EssentialContract, IBridge {
     /// @notice Invokes a call message on the Bridge.
     /// @param _message The call message to be invoked.
     /// @param _msgHash The hash of the message.
+    /// @param _checkGasleft True to check gasleft is sufficient for target function invocation.
     /// @return success_ A boolean value indicating whether the message call was successful.
     /// @dev This function updates the context in the state before and after the
     /// message call.
     function _invokeMessageCall(
         Message calldata _message,
         bytes32 _msgHash,
-        uint256 _gasLimit
+        uint256 _gasLimit,
+        bool _checkGasleft
     )
         private
         returns (bool success_)
@@ -493,6 +501,9 @@ contract Bridge is EssentialContract, IBridge {
         if (_gasLimit == 0) return false;
 
         _storeContext(_msgHash, _message.from, _message.srcChainId);
+        if (_checkGasleft && _hasInsufficientGas(_gasLimit, _message.data.length)) {
+            revert B_INSUFFICIENT_GAS();
+        }
         success_ = _message.to.sendEther(_message.value, _gasLimit, _message.data);
         _resetContext();
     }
@@ -535,24 +546,6 @@ contract Bridge is EssentialContract, IBridge {
         }
     }
 
-    /// @notice Loads and returns the call context.
-    /// @return ctx_ The call context.
-    function _loadContext() private view returns (Context memory) {
-        if (LibNetwork.isDencunSupported(block.chainid)) {
-            bytes32 msgHash;
-            address from;
-            uint64 srcChainId;
-            assembly {
-                msgHash := tload(_CTX_SLOT)
-                from := tload(add(_CTX_SLOT, 1))
-                srcChainId := tload(add(_CTX_SLOT, 2))
-            }
-            return Context(msgHash, from, srcChainId);
-        } else {
-            return __ctx;
-        }
-    }
-
     /// @notice Checks if the signal was received and caches cross-chain data if requested.
     /// @param _signalService The signal service address.
     /// @param _signal The signal.
@@ -577,6 +570,39 @@ contract Bridge is EssentialContract, IBridge {
         }
     }
 
+    /// @notice Consumes a given amount of Ether from quota manager.
+    /// @param _amount The amount of Ether to consume.
+    /// @return true if quota manager has unlimited quota for Ether or the given amount of Ether is
+    /// consumed already.
+    function _consumeEtherQuota(uint256 _amount) private returns (bool) {
+        address quotaManager = resolve(LibStrings.B_QUOTA_MANAGER, true);
+        if (quotaManager == address(0)) return true;
+
+        try IQuotaManager(quotaManager).consumeQuota(address(0), _amount) {
+            return true;
+        } catch {
+            return false;
+        }
+    }
+
+    /// @notice Loads and returns the call context.
+    /// @return ctx_ The call context.
+    function _loadContext() private view returns (Context memory) {
+        if (LibNetwork.isDencunSupported(block.chainid)) {
+            bytes32 msgHash;
+            address from;
+            uint64 srcChainId;
+            assembly {
+                msgHash := tload(_CTX_SLOT)
+                from := tload(add(_CTX_SLOT, 1))
+                srcChainId := tload(add(_CTX_SLOT, 2))
+            }
+            return Context(msgHash, from, srcChainId);
+        } else {
+            return __ctx;
+        }
+    }
+
     /// @notice Checks if the signal was received.
     /// This is the 'readonly' version of _proveSignalReceived.
     /// @param _signalService The signal service address.
@@ -603,44 +629,38 @@ contract Bridge is EssentialContract, IBridge {
         }
     }
 
-    function _invocationGasLimit(
-        Message calldata _message,
-        bool _checkThe63Over64Rule
+    ///@dev This implementation was suggested by OpenZeppelin in an audit to replace the previous.
+    function _hasInsufficientGas(
+        uint256 _minGas,
+        uint256 _dataLength
     )
         private
         view
-        returns (uint256 gasLimit_)
+        returns (bool result_)
     {
         unchecked {
-            uint256 minGasRequired = getMessageMinGasLimit(_message.data.length);
-            gasLimit_ = minGasRequired.max(_message.gasLimit) - minGasRequired;
-        }
+            // https://github.com/ethereum/execution-specs/blob/master/src/ethereum/cancun/vm/gas.py#L128
+            uint256 words = _dataLength / 32 + 1;
+            uint256 memoryGasCost = words * 3 + words * words / 512;
 
-        if (_checkThe63Over64Rule && (gasleft() * 63) >> 6 < gasLimit_) {
-            revert B_INSUFFICIENT_GAS();
+            // Need to add the gas cost of accessing the message.to account. This currently
+            // corresponds to 2600 gas if the account is cold, and 100 otherwise.
+
+            // Also need to add the cost of transferring a nonzero msg.value. This cost is currently
+            // 9000, but provides a 2300 gas stipend to the called contract.
+            result_ = gasleft() * 63 < _minGas * 64 + (memoryGasCost + 9300) * 63;
         }
     }
 
     function _checkStatus(bytes32 _msgHash, Status _expectedStatus) private view {
         if (messageStatus[_msgHash] != _expectedStatus) revert B_INVALID_STATUS();
     }
 
-    function _consumeEtherQuota(uint256 _amount) private returns (bool) {
-        address quotaManager = resolve(LibStrings.B_QUOTA_MANAGER, true);
-        if (quotaManager == address(0)) return true;
-
-        try IQuotaManager(quotaManager).consumeQuota(address(0), _amount) {
-            return true;
-        } catch {
-            return false;
-        }
-    }
-
     function _unableToInvokeMessageCall(
         Message calldata _message,
         address _signalService
     )
-        internal
+        private
         view
         returns (bool)
     {
@@ -652,4 +672,11 @@ contract Bridge is EssentialContract, IBridge {
             && bytes4(_message.data) != IMessageInvocable.onMessageInvocation.selector
             && _message.to.isContract();
     }
+
+    function _invocationGasLimit(Message calldata _message) private pure returns (uint256) {
+        uint256 minGasRequired = getMessageMinGasLimit(_message.data.length);
+        unchecked {
+            return minGasRequired.max(_message.gasLimit) - minGasRequired;
+        }
+    }
 }

packages/protocol/deployments/mainnet-contract-logs-L1.md

@@ -74,7 +74,7 @@
   - upgraded from `0xc71CC3B0a47149878fad337fb2ca54E546A645ba` to `0x02F21B4C3d4dbfF70cE851741175a727c8D782Be` @commit`fa481c1` in @tx`0x02ed558762eae5f0a930ba4a1047a02d4a793ea48890268c32df04e882f138ff`
   - unpaused on 27 May, 2024 @tx`0x71ce1e61f1e42e34c9a51f5671ac260f2ac398e016ae645f2661f074e7f230ce`
   - upgraded from `0x02F21B4C3d4dbfF70cE851741175a727c8D782Be` to `0x71c2f41AEDe913AAEf2c62596E03702E348D6Cd0.` @commit`` in @tx`0x8a380a25d03a740d9535dfc3e2fc4f6960e22d49ad88b8d85f59af4013aedf87`
-  - upgrade impl to `0x951B7Ae1bB26d12dB37f01748e8fB62FEf45A8B5` @tx`0xf21f6bf720767db3bc9b63ef69cacb20340bdedfb6589e6a4d11fe082dfa7bd6`
+  - upgrade impl to `0x951B7Ae1bB26d12dB37f01748e8fB62FEf45A8B5` @commit`1bd3285` @tx`0xf21f6bf720767db3bc9b63ef69cacb20340bdedfb6589e6a4d11fe082dfa7bd6`
 
 #### quota_manager
 
@@ -316,9 +316,9 @@
   - upgraded from `0x68d30f47F19c07bCCEf4Ac7FAE2Dc12FCa3e0dC9` to `0x500735343372Dd6c9B84dBc7a75babf4479742B9` @commit`fa481c1` @tx`0x02ed558762eae5f0a930ba4a1047a02d4a793ea48890268c32df04e882f138ff`
   - disable a prover (`0x00000027F51a57E7FcBC4b481d15fcE5BE68b30B`) on May 28 @commit`b335b70` @tx`0x27c84a1dbf80d88948f96f1536c244816543fb780c81a04ba485c4c156431112`
 
-### labconster.taiko.eth
+### labcontester.taiko.eth
 
-- ens: `labconster.taiko.eth`
+- ens: `labcontester.taiko.eth`
 - proxy: `0xa01d464ca3982DAa97B19fa7F8a232eB11A9DDb3`
 - impl: `0x500735343372Dd6c9B84dBc7a75babf4479742B9`
 - enabled provers:

packages/protocol/deployments/mainnet-contract-logs-L2.md

@@ -52,7 +52,7 @@
   - upgrade to `0xf961854D68368cFFc86d90AEe8a19E9781dACA3e` @tx`0x094dd9452d79cbd74711f2b8065566e4431a05d0727c56d2b38195e40fd62805`
   - upgraded from `0xf961854D68368cFFc86d90AEe8a19E9781dACA3e` to `0x98C5De7670aA7d47C6c0551fAD27Bfe464A6751a..` @commit`` in @tx`0x0b5d6acc9c5b8ef193920246081ec5ce7268111acfc1dce1f058bea06f3953c7`
   - changed owner to `0xCa5b76Cc7A38b86Db11E5aE5B1fc9740c3bA3DE8` @tx`0xf68861171c602e3e75ca69e950957fcb908c7949c6df9a9ea3026c238ebb1e9c`
-  - upgrade impl to `0x0893c8821Fa358D5f3630695Ce062204814359A1` @tx`0x4605c4ce594e996bdbdb532a9aefe4fab1ea36f7e2ef63eef56a7e8033810df3`
+  - upgrade impl to `0x0893c8821Fa358D5f3630695Ce062204814359A1` @commit`1bd3285` @tx`0x4605c4ce594e996bdbdb532a9aefe4fab1ea36f7e2ef63eef56a7e8033810df3`
 
 #### erc20_vault