Skip to content

Commit

Permalink
CCIP-4659 New Modified ERC165Checker library (#15743)
Browse files Browse the repository at this point in the history
* New Modified ERC165Checker library

* [Bot] Update changeset file with jira issues

* fix gas snapshot

* Update gethwrappers

* fill in coverage gaps

* so many snapshots i might as well open a photography studio

* formatting

* change name for specificity and update tests, and change gas check to assembl

* Update gethwrappers

* fix snapshot issues from duplicate tests

* remove unnec. functions and better format library to conform to CL style guide.

* Update gethwrappers

* Optimize library with better boolean logic.

* formatting

* Update gethwrappers

* fix compiler version, snapshot, and wrappers broke in merge conflict resolution

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 7743429 commit 4a19318
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 56 deletions.
10 changes: 10 additions & 0 deletions contracts/.changeset/violet-lamps-pump.md
Original file line number Diff line number Diff line change
@@ -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
101 changes: 51 additions & 50 deletions contracts/gas-snapshots/ccip.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ CCIPHome_setCandidate:test_setCandidate() (gas: 1365392)
CCIPHome_supportsInterface:test_supportsInterface() (gas: 9885)
DefensiveExampleTest:test_HappyPath() (gas: 200517)
DefensiveExampleTest:test_Recovery() (gas: 424996)
E2E:test_E2E_3MessagesMMultiOffRampSuccess_gas() (gas: 1489913)
E2E:test_E2E_3MessagesMMultiOffRampSuccess_gas() (gas: 1490237)
ERC165CheckerReverting_supportsInterfaceReverting:test__supportsInterfaceReverting() (gas: 10445)
EtherSenderReceiverTest_ccipReceive:test_ccipReceive_fallbackToWethTransfer() (gas: 96964)
EtherSenderReceiverTest_ccipReceive:test_ccipReceive_happyPath() (gas: 49797)
EtherSenderReceiverTest_ccipReceive:test_ccipReceive_wrongToken() (gas: 17460)
Expand Down Expand Up @@ -195,11 +196,11 @@ NonceManager_applyPreviousRampsUpdates:test_MultipleRampsUpdates() (gas: 123595)
NonceManager_applyPreviousRampsUpdates:test_PreviousRampAlreadySet_overrideAllowed() (gas: 45935)
NonceManager_applyPreviousRampsUpdates:test_SingleRampUpdate() (gas: 66937)
NonceManager_applyPreviousRampsUpdates:test_ZeroInput() (gas: 12145)
NonceManager_getInboundNonce:test_getInboundNonce_NoPrevOffRampForChain() (gas: 178870)
NonceManager_getInboundNonce:test_getInboundNonce_Upgraded() (gas: 146059)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceNewSenderStartsAtZero() (gas: 182340)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceStartsAtV1Nonce() (gas: 245017)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedOffRampNonceSkipsIfMsgInFlight() (gas: 213699)
NonceManager_getInboundNonce:test_getInboundNonce_NoPrevOffRampForChain() (gas: 178906)
NonceManager_getInboundNonce:test_getInboundNonce_Upgraded() (gas: 146095)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceNewSenderStartsAtZero() (gas: 182376)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceStartsAtV1Nonce() (gas: 245089)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedOffRampNonceSkipsIfMsgInFlight() (gas: 213735)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedSenderNoncesReadsPreviousRamp() (gas: 60418)
NonceManager_getIncrementedOutboundNonce:test_getIncrementedOutboundNonce() (gas: 37974)
NonceManager_getIncrementedOutboundNonce:test_incrementInboundNonce() (gas: 38746)
Expand All @@ -215,12 +216,12 @@ OffRamp_applySourceChainConfigUpdates:test_ApplyZeroUpdates() (gas: 16671)
OffRamp_applySourceChainConfigUpdates:test_ReplaceExistingChain() (gas: 180998)
OffRamp_applySourceChainConfigUpdates:test_ReplaceExistingChainOnRamp() (gas: 168513)
OffRamp_applySourceChainConfigUpdates:test_allowNonOnRampUpdateAfterLaneIsUsed() (gas: 284861)
OffRamp_batchExecute:test_MultipleReportsDifferentChains() (gas: 325920)
OffRamp_batchExecute:test_MultipleReportsDifferentChainsSkipCursedChain() (gas: 170816)
OffRamp_batchExecute:test_MultipleReportsSameChain() (gas: 269219)
OffRamp_batchExecute:test_MultipleReportsSkipDuplicate() (gas: 161775)
OffRamp_batchExecute:test_SingleReport() (gas: 149581)
OffRamp_batchExecute:test_Unhealthy() (gas: 533110)
OffRamp_batchExecute:test_MultipleReportsDifferentChains() (gas: 326028)
OffRamp_batchExecute:test_MultipleReportsDifferentChainsSkipCursedChain() (gas: 170852)
OffRamp_batchExecute:test_MultipleReportsSameChain() (gas: 269327)
OffRamp_batchExecute:test_MultipleReportsSkipDuplicate() (gas: 161811)
OffRamp_batchExecute:test_SingleReport() (gas: 149617)
OffRamp_batchExecute:test_Unhealthy() (gas: 533326)
OffRamp_commit:test_OnlyGasPriceUpdates() (gas: 112973)
OffRamp_commit:test_OnlyTokenPriceUpdates() (gas: 112927)
OffRamp_commit:test_PriceSequenceNumberCleared() (gas: 355397)
Expand All @@ -229,51 +230,51 @@ OffRamp_commit:test_ReportOnlyRootSuccess_gas() (gas: 141051)
OffRamp_commit:test_RootWithRMNDisabled() (gas: 153873)
OffRamp_commit:test_StaleReportWithRoot() (gas: 232101)
OffRamp_commit:test_ValidPriceUpdateThenStaleReportWithRoot() (gas: 206722)
OffRamp_constructor:test_Constructor() (gas: 6268112)
OffRamp_execute:test_LargeBatch() (gas: 3372780)
OffRamp_execute:test_MultipleReports() (gas: 291350)
OffRamp_execute:test_MultipleReportsWithPartialValidationFailures() (gas: 364740)
OffRamp_execute:test_SingleReport() (gas: 168814)
OffRamp_executeSingleMessage:test_executeSingleMessage_NoTokens() (gas: 51574)
OffRamp_constructor:test_Constructor() (gas: 6269512)
OffRamp_execute:test_LargeBatch() (gas: 3373860)
OffRamp_execute:test_MultipleReports() (gas: 291458)
OffRamp_execute:test_MultipleReportsWithPartialValidationFailures() (gas: 364776)
OffRamp_execute:test_SingleReport() (gas: 168850)
OffRamp_executeSingleMessage:test_executeSingleMessage_NoTokens() (gas: 51610)
OffRamp_executeSingleMessage:test_executeSingleMessage_NonContract() (gas: 20514)
OffRamp_executeSingleMessage:test_executeSingleMessage_NonContractWithTokens() (gas: 230346)
OffRamp_executeSingleMessage:test_executeSingleMessage_WithMessageInterceptor() (gas: 87429)
OffRamp_executeSingleMessage:test_executeSingleMessage_WithTokens() (gas: 259827)
OffRamp_executeSingleReport:test_InvalidSourcePoolAddress() (gas: 455214)
OffRamp_executeSingleReport:test_ReceiverError() (gas: 180761)
OffRamp_executeSingleReport:test_SingleMessageNoTokens() (gas: 205198)
OffRamp_executeSingleReport:test_SingleMessageNoTokensOtherChain() (gas: 241285)
OffRamp_executeSingleReport:test_SingleMessageNoTokensUnordered() (gas: 185191)
OffRamp_executeSingleReport:test_SingleMessageToNonCCIPReceiver() (gas: 243935)
OffRamp_executeSingleReport:test_SingleMessagesNoTokensSuccess_gas() (gas: 134620)
OffRamp_executeSingleMessage:test_executeSingleMessage_NonContractWithTokens() (gas: 230418)
OffRamp_executeSingleMessage:test_executeSingleMessage_WithMessageInterceptor() (gas: 87465)
OffRamp_executeSingleMessage:test_executeSingleMessage_WithTokens() (gas: 259935)
OffRamp_executeSingleReport:test_InvalidSourcePoolAddress() (gas: 455358)
OffRamp_executeSingleReport:test_ReceiverError() (gas: 180797)
OffRamp_executeSingleReport:test_SingleMessageNoTokens() (gas: 205270)
OffRamp_executeSingleReport:test_SingleMessageNoTokensOtherChain() (gas: 241357)
OffRamp_executeSingleReport:test_SingleMessageNoTokensUnordered() (gas: 185263)
OffRamp_executeSingleReport:test_SingleMessageToNonCCIPReceiver() (gas: 243920)
OffRamp_executeSingleReport:test_SingleMessagesNoTokensSuccess_gas() (gas: 134656)
OffRamp_executeSingleReport:test_SkippedIncorrectNonce() (gas: 58298)
OffRamp_executeSingleReport:test_SkippedIncorrectNonceStillExecutes() (gas: 392286)
OffRamp_executeSingleReport:test_TwoMessagesWithTokensAndGE() (gas: 562211)
OffRamp_executeSingleReport:test_TwoMessagesWithTokensSuccess_gas() (gas: 510592)
OffRamp_executeSingleReport:test_Unhealthy() (gas: 528733)
OffRamp_executeSingleReport:test_WithCurseOnAnotherSourceChain() (gas: 439083)
OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessage() (gas: 158002)
OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessageUnordered() (gas: 128386)
OffRamp_executeSingleReport:test_SkippedIncorrectNonceStillExecutes() (gas: 392394)
OffRamp_executeSingleReport:test_TwoMessagesWithTokensAndGE() (gas: 562427)
OffRamp_executeSingleReport:test_TwoMessagesWithTokensSuccess_gas() (gas: 510808)
OffRamp_executeSingleReport:test_Unhealthy() (gas: 528949)
OffRamp_executeSingleReport:test_WithCurseOnAnotherSourceChain() (gas: 439299)
OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessage() (gas: 158038)
OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessageUnordered() (gas: 128422)
OffRamp_getExecutionState:test_FillExecutionState() (gas: 3955662)
OffRamp_getExecutionState:test_GetDifferentChainExecutionState() (gas: 121311)
OffRamp_getExecutionState:test_GetExecutionState() (gas: 90102)
OffRamp_manuallyExecute:test_manuallyExecute() (gas: 212296)
OffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched() (gas: 165706)
OffRamp_manuallyExecute:test_manuallyExecute_LowGasLimit() (gas: 479073)
OffRamp_manuallyExecute:test_manuallyExecute_ReentrancyFails() (gas: 2229590)
OffRamp_manuallyExecute:test_manuallyExecute_WithGasOverride() (gas: 212846)
OffRamp_manuallyExecute:test_manuallyExecute_WithMultiReportGasOverride() (gas: 731858)
OffRamp_manuallyExecute:test_manuallyExecute_WithPartialMessages() (gas: 336871)
OffRamp_releaseOrMintSingleToken:test__releaseOrMintSingleToken() (gas: 94593)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens() (gas: 161085)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_WithGasOverride() (gas: 162951)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_destDenominatedDecimals() (gas: 174204)
OffRamp_manuallyExecute:test_manuallyExecute() (gas: 212368)
OffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched() (gas: 165742)
OffRamp_manuallyExecute:test_manuallyExecute_LowGasLimit() (gas: 479145)
OffRamp_manuallyExecute:test_manuallyExecute_ReentrancyFails() (gas: 2229662)
OffRamp_manuallyExecute:test_manuallyExecute_WithGasOverride() (gas: 212918)
OffRamp_manuallyExecute:test_manuallyExecute_WithMultiReportGasOverride() (gas: 732218)
OffRamp_manuallyExecute:test_manuallyExecute_WithPartialMessages() (gas: 337015)
OffRamp_releaseOrMintSingleToken:test__releaseOrMintSingleToken() (gas: 94629)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens() (gas: 161157)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_WithGasOverride() (gas: 163023)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_destDenominatedDecimals() (gas: 174276)
OffRamp_setDynamicConfig:test_SetDynamicConfig() (gas: 25442)
OffRamp_setDynamicConfig:test_SetDynamicConfigWithInterceptor() (gas: 47493)
OffRamp_trialExecute:test_trialExecute() (gas: 263527)
OffRamp_trialExecute:test_trialExecute_RateLimitError() (gas: 120685)
OffRamp_trialExecute:test_trialExecute_TokenHandlingErrorIsCaught() (gas: 131995)
OffRamp_trialExecute:test_trialExecute_TokenPoolIsNotAContract() (gas: 281272)
OffRamp_trialExecute:test_trialExecute() (gas: 263635)
OffRamp_trialExecute:test_trialExecute_RateLimitError() (gas: 120721)
OffRamp_trialExecute:test_trialExecute_TokenHandlingErrorIsCaught() (gas: 132031)
OffRamp_trialExecute:test_trialExecute_TokenPoolIsNotAContract() (gas: 281380)
OnRampTokenPoolReentrancy:test_OnRampTokenPoolReentrancy() (gas: 244294)
OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates() (gas: 325979)
OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates_InvalidAllowListRequestDisabledAllowListWithAdds() (gas: 17190)
Expand Down
65 changes: 65 additions & 0 deletions contracts/src/v0.8/ccip/libraries/ERC165CheckerReverting.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import {IERC165} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/IERC165.sol";

/// @notice Library used to query support of an interface declared via {IERC165}.
/// @dev These functions return the actual result of the query: they do not `revert` if an interface is not supported.
library ERC165CheckerReverting {
error InsufficientGasForStaticCall();

// As per the EIP-165 spec, no interface should ever match 0xffffffff.
bytes4 private constant INTERFACE_ID_INVALID = 0xffffffff;

/// @dev 30k gas is required to make the staticcall. Under the 63/64 rule this means that 30,477 gas must be available
/// to ensure that at least 30k is forwarded. Checking for at least 31,000 ensures that after additional
/// operations are performed there is still >= 30,477 gas remaining.
/// 30,000 = ((30,477 * 63) / 64)
uint256 private constant MINIMUM_GAS_REQUIREMENT = 31_000;

/// @notice Returns true if `account` supports a defined interface.
/// @dev The function must support both the interfaceId and interfaces specified by ERC165 generally as per the standard.
/// @param account the contract to be queried for support.
/// @param interfaceId the interface being checked for support.
/// @return true if the contract at account indicates support of the interface with, false otherwise.
function _supportsInterfaceReverting(address account, bytes4 interfaceId) internal view returns (bool) {
// As a gas optimization, short circuit return false if interfaceId is not supported, as it is most likely interfaceId
// to be unsupported by the target.
return _supportsERC165InterfaceUncheckedReverting(account, interfaceId)
&& !_supportsERC165InterfaceUncheckedReverting(account, INTERFACE_ID_INVALID)
&& _supportsERC165InterfaceUncheckedReverting(account, type(IERC165).interfaceId);
}

/// @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.
/// @dev Function will only revert if the minimum gas requirement is not met before the staticcall is performed.
function _supportsERC165InterfaceUncheckedReverting(address account, bytes4 interfaceId) internal view returns (bool) {
bytes memory encodedParams = abi.encodeWithSelector(IERC165.supportsInterface.selector, interfaceId);

bool success;
uint256 returnSize;
uint256 returnValue;

bytes4 notEnoughGasSelector = InsufficientGasForStaticCall.selector;

assembly {
// The EVM does not return a specific error code if a revert is due to OOG. This check ensures that
// the message will not throw an OOG error by requiring that the amount of gas for the following
// staticcall exists before invoking it.
if lt(gas(), MINIMUM_GAS_REQUIREMENT) {
mstore(0x0, notEnoughGasSelector)
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;
}
}
11 changes: 7 additions & 4 deletions contracts/src/v0.8/ccip/offRamp/OffRamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
Expand Down Expand Up @@ -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
// 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]
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity ^0.8.24;

import {IAny2EVMMessageReceiver} from "../../interfaces/IAny2EVMMessageReceiver.sol";

import {ERC165CheckerReverting} from "../../libraries/ERC165CheckerReverting.sol";
import {MaybeRevertMessageReceiver} from "../helpers/receivers/MaybeRevertMessageReceiver.sol";

import {Test} from "forge-std/Test.sol";

contract ERC165CheckerReverting_supportsInterfaceReverting is Test {
using ERC165CheckerReverting for address;

address internal s_receiver;

bytes4 internal constant EXAMPLE_INTERFACE_ID = 0xdeadbeef;

error InsufficientGasForStaticCall();

constructor() {
s_receiver = address(new MaybeRevertMessageReceiver(false));
}

function test__supportsInterfaceReverting() public view {
assertTrue(s_receiver._supportsInterfaceReverting(type(IAny2EVMMessageReceiver).interfaceId));
}

// Reverts

function test__supportsInterfaceReverting_RevertWhen_NotEnoughGasForSupportsInterface() public {
vm.expectRevert(InsufficientGasForStaticCall.selector);

// 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, <31k would remain to trigger the error.
this.invokeERC165Checker{gas: 33_000}();
}

// Meant to test the call with a manual gas limit override
function invokeERC165Checker() external view {
s_receiver._supportsInterfaceReverting(EXAMPLE_INTERFACE_ID);
}
}
2 changes: 1 addition & 1 deletion core/gethwrappers/ccip/generated/offramp/offramp.go

Large diffs are not rendered by default.

Loading

0 comments on commit 4a19318

Please sign in to comment.