CCIP-4659 New Modified ERC165Checker library

contracts/.changeset/violet-lamps-pump.md

@@ -0,0 +1,10 @@
+---
+'@chainlink/contracts': patch
+---
+
+Create a new version of the ERC165Checker library which checks for sufficient gas before making an external call to prevent message delivery issues. #bugfix
+
+
+PR issue: CCIP-4659
+
+Solidity Review issue: CCIP-3966

contracts/src/v0.8/ccip/libraries/ERC165CheckerReverting.sol

@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import {IERC165} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/IERC165.sol";
+
+/**
+ * @dev Library used to query support of an interface declared via {IERC165}.
+ *
+ * Note that these functions return the actual result of the query: they do not
+ * `revert` if an interface is not supported. It is up to the caller to decide
+ * what to do in these cases.
+ *
+ * Note this is exactly the same as the OZ version, with the exception that external calls will revert
+ * if < 31_000 gas is available, to prevent message delivery issues in CCIP.
+ */
+library ERC165CheckerReverting {
+  // As per the EIP-165 spec, no interface should ever match 0xffffffff
+  bytes4 private constant _INTERFACE_ID_INVALID = 0xffffffff;
+
+  // bytes4(keccak256("NotEnoughGasForSupportsInterfaceCall()"))
+  bytes4 private constant NOT_ENOUGH_GAS_SIG = 0x161c3bf7;
+
+  /**
+   * @dev Returns true if `account` supports the {IERC165} interface.
+   */
+  function _supportsERC165Reverting(
+    address account
+  ) internal view returns (bool) {
+    // Any contract that implements ERC165 must explicitly indicate support of
+    // InterfaceId_ERC165 and explicitly indicate non-support of InterfaceId_Invalid
+    return _supportsERC165InterfaceUncheckedReverting(account, type(IERC165).interfaceId)
+      && !_supportsERC165InterfaceUncheckedReverting(account, _INTERFACE_ID_INVALID);
+  }
+
+  /**
+   * @dev Returns true if `account` supports the interface defined by
+   * `interfaceId`. Support for {IERC165} itself is queried automatically.
+   *
+   * See {IERC165-supportsInterface}.
+   */
+  function _supportsInterfaceReverting(address account, bytes4 interfaceId) internal view returns (bool) {
+    // query support of both ERC165 as per the spec and support of _interfaceId
+    return _supportsERC165Reverting(account) && _supportsERC165InterfaceUncheckedReverting(account, interfaceId);
+  }
+
+  /**
+   * @dev Returns a boolean array where each value corresponds to the
+   * interfaces passed in and whether they're supported or not. This allows
+   * you to batch check interfaces for a contract where your expectation
+   * is that some interfaces may not be supported.
+   *
+   * See {IERC165-supportsInterface}.
+   *
+   * _Available since v3.4._
+   */
+  function _getSupportedInterfacesReverting(
+    address account,
+    bytes4[] memory interfaceIds
+  ) internal view returns (bool[] memory) {
+    // an array of booleans corresponding to interfaceIds and whether they're supported or not
+    bool[] memory interfaceIdsSupported = new bool[](interfaceIds.length);
+
+    // query support of ERC165 itself
+    if (_supportsERC165Reverting(account)) {
+      // query support of each interface in interfaceIds
+      for (uint256 i = 0; i < interfaceIds.length; i++) {
+        interfaceIdsSupported[i] = _supportsERC165InterfaceUncheckedReverting(account, interfaceIds[i]);
+      }
+    }
+
+    return interfaceIdsSupported;
+  }
+
+  /**
+   * @dev Returns true if `account` supports all the interfaces defined in
+   * `interfaceIds`. Support for {IERC165} itself is queried automatically.
+   *
+   * Batch-querying can lead to gas savings by skipping repeated checks for
+   * {IERC165} support.
+   *
+   * See {IERC165-supportsInterface}.
+   */
+  function _supportsAllInterfacesReverting(address account, bytes4[] memory interfaceIds) internal view returns (bool) {
+    // query support of ERC165 itself
+    if (!_supportsERC165Reverting(account)) {
+      return false;
+    }
+
+    // query support of each interface in interfaceIds
+    for (uint256 i = 0; i < interfaceIds.length; i++) {
+      if (!_supportsERC165InterfaceUncheckedReverting(account, interfaceIds[i])) {
+        return false;
+      }
+    }
+
+    // all interfaces supported
+    return true;
+  }
+
+  /**
+   * @notice Query if a contract implements an interface, does not check ERC165 support
+   * @param account The address of the contract to query for support of an interface
+   * @param interfaceId The interface identifier, as specified in ERC-165
+   * @return true if the contract at account indicates support of the interface with
+   * identifier interfaceId, false otherwise
+   * @dev Assumes that account contains a contract that supports ERC165, otherwise
+   * the behavior of this method is undefined. This precondition can be checked
+   * with {supportsERC165}.
+   *
+   * Some precompiled contracts will falsely indicate support for a given interface, so caution
+   * should be exercised when using this function.
+   *
+   * Interface identification is specified in ERC-165.
+   */
+  function _supportsERC165InterfaceUncheckedReverting(address account, bytes4 interfaceId) internal view returns (bool) {
+    // prepare call
+    bytes memory encodedParams = abi.encodeWithSelector(IERC165.supportsInterface.selector, interfaceId);
+
+    // perform static call
+    bool success;
+    uint256 returnSize;
+    uint256 returnValue;
+
+    assembly {
+      // Enforce that there's enough gas avilable so that the call will not fail due to OOG error. Without this supportsInterface() may return false when it should return true.
+
+      // 32,000 gas was chosen to ensure enough gas to invoke the staticcall after the check
+      // without breaking the 63/64 rule. Under EIP-150 there must be at least ~30,476 gas
+      // remaining before the staticcall.
+      if lt(gas(), 32000) {
+        mstore(0x0, NOT_ENOUGH_GAS_SIG)
+        revert(0x0, 0x4)
+      }
+
+      success := staticcall(30000, account, add(encodedParams, 0x20), mload(encodedParams), 0x00, 0x20)
+      returnSize := returndatasize()
+      returnValue := mload(0x00)
+    }
+    return success && returnSize >= 0x20 && returnValue > 0;
+  }
+}

contracts/src/v0.8/ccip/offRamp/OffRamp.sol

@@ -13,13 +13,13 @@ import {ITokenAdminRegistry} from "../interfaces/ITokenAdminRegistry.sol";
 
 import {CallWithExactGas} from "../../shared/call/CallWithExactGas.sol";
 import {Client} from "../libraries/Client.sol";
+import {ERC165CheckerReverting} from "../libraries/ERC165CheckerReverting.sol";
 import {Internal} from "../libraries/Internal.sol";
 import {MerkleMultiProof} from "../libraries/MerkleMultiProof.sol";
 import {Pool} from "../libraries/Pool.sol";
 import {MultiOCR3Base} from "../ocr/MultiOCR3Base.sol";
 
 import {IERC20} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/token/ERC20/IERC20.sol";
-import {ERC165Checker} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/introspection/ERC165Checker.sol";
 import {EnumerableSet} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/structs/EnumerableSet.sol";
 
 /// @notice OffRamp enables OCR networks to execute multiple messages in an OffRamp in a single transaction.
@@ -28,7 +28,7 @@ import {EnumerableSet} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts
 /// @dev MultiOCR3Base is used to store multiple OCR configs for the OffRamp. The execution plugin type has to be
 /// configured without signature verification, and the commit plugin type with verification.
 contract OffRamp is ITypeAndVersion, MultiOCR3Base {
-  using ERC165Checker for address;
+  using ERC165CheckerReverting for address;
   using EnumerableSet for EnumerableSet.UintSet;
 
   error ZeroChainSelectorNotAllowed();
@@ -604,9 +604,12 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
     // 3. If the receiver is a contract but it does not support the IAny2EVMMessageReceiver interface.
     //
     // The ordering of these checks is important, as the first check is the cheapest to execute.
+    //
+    // To prevent message delivery bypass issues, a modified version of the ERC165Checker is used exclusively here
+    // which checks for sufficient gas before making the external call.
     if (
       (message.data.length == 0 && message.gasLimit == 0) || message.receiver.code.length == 0
-        || !message.receiver.supportsInterface(type(IAny2EVMMessageReceiver).interfaceId)
+        || !message.receiver._supportsInterfaceReverting(type(IAny2EVMMessageReceiver).interfaceId)
     ) return;
 
     (bool success, bytes memory returnData,) = s_sourceChainConfigs[message.header.sourceChainSelector]
@@ -647,7 +650,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
     // This is done to prevent a pool from reverting the entire transaction if it doesn't support the interface.
     // The call gets a max or 30k gas per instance, of which there are three. This means offchain gas estimations should
     // account for 90k gas overhead due to the interface check.
-    if (localPoolAddress == address(0) || !localPoolAddress.supportsInterface(Pool.CCIP_POOL_V1)) {
+    if (localPoolAddress == address(0) || !localPoolAddress._supportsInterfaceReverting(Pool.CCIP_POOL_V1)) {
       revert NotACompatiblePool(localPoolAddress);
     }
 

contracts/src/v0.8/ccip/test/libraries/ERC165CheckerReverting/ERC165CheckerReverting.getSupportedInterfacesReverting.t.sol

@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity 0.8.24;
+
+import {IAny2EVMMessageReceiver} from "../../../interfaces/IAny2EVMMessageReceiver.sol";
+
+import {ERC165CheckerReverting} from "../../../libraries/ERC165CheckerReverting.sol";
+import {ERC165CheckerRevertingSetup} from "./ERC165CheckerRevertingSetup.t.sol";
+
+contract ERC165CheckerReverting_getSupportedInterfaces is ERC165CheckerRevertingSetup {
+  using ERC165CheckerReverting for address;
+
+  function test__getSupportedInterfacesReverting() public view {
+    bytes4[] memory interfaceIds = new bytes4[](1);
+
+    interfaceIds[0] = type(IAny2EVMMessageReceiver).interfaceId;
+
+    bool[] memory supportedIds = s_receiver._getSupportedInterfacesReverting(interfaceIds);
+    assertTrue(supportedIds[0]);
+    assertEq(interfaceIds.length, supportedIds.length);
+  }
+}

contracts/src/v0.8/ccip/test/libraries/ERC165CheckerReverting/ERC165CheckerReverting.supportsAllInterfacesReverting.t.sol

@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity 0.8.24;
+
+import {IAny2EVMMessageReceiver} from "../../../interfaces/IAny2EVMMessageReceiver.sol";
+
+import {ERC165CheckerReverting} from "../../../libraries/ERC165CheckerReverting.sol";
+import {ERC165CheckerRevertingSetup} from "./ERC165CheckerRevertingSetup.t.sol";
+
+contract ERC165CheckerReverting_supportsInterfaceReverting is ERC165CheckerRevertingSetup {
+  using ERC165CheckerReverting for address;
+
+  function test__getSupportedInterfacesReverting() public view {
+    bytes4[] memory interfaceIds = new bytes4[](1);
+
+    interfaceIds[0] = type(IAny2EVMMessageReceiver).interfaceId;
+
+    bool[] memory supportedIds = s_receiver._getSupportedInterfacesReverting(interfaceIds);
+    assertTrue(supportedIds[0]);
+    assertEq(interfaceIds.length, supportedIds.length);
+  }
+
+  function test__supportsAllInterfacesReverting() public view {
+    bytes4[] memory interfaceIds = new bytes4[](1);
+
+    interfaceIds[0] = type(IAny2EVMMessageReceiver).interfaceId;
+
+    assertTrue(s_receiver._supportsAllInterfacesReverting(interfaceIds));
+  }
+
+  function test__supportsAllInterfacesReverting_notAllSupported() public view {
+    bytes4[] memory interfaceIds = new bytes4[](2);
+
+    interfaceIds[0] = type(IAny2EVMMessageReceiver).interfaceId;
+    interfaceIds[1] = EXAMPLE_INTERFACE_ID;
+
+    assertFalse(s_receiver._supportsAllInterfacesReverting(interfaceIds));
+  }
+
+  // Reverts
+
+  function test__supportsAllInterfacesReverting_notSupportsERC165() public view {
+    bytes4[] memory interfaceIds = new bytes4[](2);
+
+    interfaceIds[0] = type(IAny2EVMMessageReceiver).interfaceId;
+    interfaceIds[1] = EXAMPLE_INTERFACE_ID;
+
+    // An address that does not support ERC165
+    address randomAddress = address(0xdead);
+
+    assertFalse(randomAddress._supportsAllInterfacesReverting(interfaceIds));
+  }
+}

contracts/src/v0.8/ccip/test/libraries/ERC165CheckerReverting/ERC165CheckerReverting.supportsInterfaceReverting.t.sol

@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity 0.8.24;
+
+import {IAny2EVMMessageReceiver} from "../../../interfaces/IAny2EVMMessageReceiver.sol";
+
+import {ERC165CheckerReverting} from "../../../libraries/ERC165CheckerReverting.sol";
+import {ERC165CheckerRevertingSetup} from "./ERC165CheckerRevertingSetup.t.sol";
+
+contract ERC165CheckerReverting_supportsInterfaceReverting is ERC165CheckerRevertingSetup {
+  using ERC165CheckerReverting for address;
+
+  function test__supportsInterfaceReverting() public view {
+    assertTrue(s_receiver._supportsInterfaceReverting(type(IAny2EVMMessageReceiver).interfaceId));
+  }
+
+  // Reverts
+
+  function test__supportsInterfaceReverting_RevertWhen_NotEnoughGasForSupportsInterface() public {
+    vm.expectRevert(NOT_ENOUGH_GAS_SIG);
+
+    // Library calls cannot be called with gas limit overrides, so a public function must be exposed
+    // instead which can proxy the call to the library.
+
+    // The gas limit was chosen so that after overhead, <30k would remain to trigger the error.
+    this.invokeERC165Checker{gas: 35_000}();
+  }
+
+  // Meant to test the call with a manual gas limit override
+  function invokeERC165Checker() external view {
+    s_receiver._supportsInterfaceReverting(EXAMPLE_INTERFACE_ID);
+  }
+}

contracts/src/v0.8/ccip/test/libraries/ERC165CheckerReverting/ERC165CheckerRevertingSetup.t.sol

@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity 0.8.24;
+
+import {MaybeRevertMessageReceiver} from "../../helpers/receivers/MaybeRevertMessageReceiver.sol";
+
+import {Test} from "forge-std/Test.sol";
+
+contract ERC165CheckerRevertingSetup is Test {
+  address internal s_receiver;
+
+  bytes4 internal constant EXAMPLE_INTERFACE_ID = 0xdeadbeef;
+  bytes4 internal constant NOT_ENOUGH_GAS_SIG = 0x161c3bf7;
+
+  constructor() {
+    s_receiver = address(new MaybeRevertMessageReceiver(false));
+  }
+}

core/gethwrappers/ccip/generation/generated-wrapper-dependency-versions-do-not-edit.txt

@@ -16,7 +16,7 @@ mock_v3_aggregator_contract: ../../../contracts/solc/v0.8.24/MockV3Aggregator/Mo
 multi_aggregate_rate_limiter: ../../../contracts/solc/v0.8.24/MultiAggregateRateLimiter/MultiAggregateRateLimiter.abi ../../../contracts/solc/v0.8.24/MultiAggregateRateLimiter/MultiAggregateRateLimiter.bin c3cac2010c2815b484055bf981363a2bd04e7fbe7bb502dc8fd29a16165d221c
 multi_ocr3_helper: ../../../contracts/solc/v0.8.24/MultiOCR3Helper/MultiOCR3Helper.abi ../../../contracts/solc/v0.8.24/MultiOCR3Helper/MultiOCR3Helper.bin a523e11ea4c069d7d61b309c156951cc6834aff0f352bd1ac37c3a838ff2588f
 nonce_manager: ../../../contracts/solc/v0.8.24/NonceManager/NonceManager.abi ../../../contracts/solc/v0.8.24/NonceManager/NonceManager.bin e6008490d916826cefd1903612db39621d51617300fc9bb42b68c6c117958198
-offramp: ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.abi ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.bin 067fdfbf7cae1557fc03ca16d9c38737ee4595655792a1b8bc4846c45caa0c74
+offramp: ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.abi ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.bin 8e6c048d3a7799f4149bd7e470064be375a35538404b30ade4798ac27f8dbf7e
 onramp: ../../../contracts/solc/v0.8.24/OnRamp/OnRamp.abi ../../../contracts/solc/v0.8.24/OnRamp/OnRamp.bin 2bf74188a997218502031f177cb2df505b272d66b25fd341a741289e77380c59
 ping_pong_demo: ../../../contracts/solc/v0.8.24/PingPongDemo/PingPongDemo.abi ../../../contracts/solc/v0.8.24/PingPongDemo/PingPongDemo.bin 24b4415a883a470d65c484be0fa20714a46b1c9262db205f1c958017820307b2
 registry_module_owner_custom: ../../../contracts/solc/v0.8.24/RegistryModuleOwnerCustom/RegistryModuleOwnerCustom.abi ../../../contracts/solc/v0.8.24/RegistryModuleOwnerCustom/RegistryModuleOwnerCustom.bin 0fc277a0b512db4e20b5a32a775b94ed2c0d342d8237511de78c94f7dacad428