Skip to content

Commit

Permalink
Merge dc54925 into 72fd6a4
Browse files Browse the repository at this point in the history
  • Loading branch information
jhweintraub authored Dec 19, 2024
2 parents 72fd6a4 + dc54925 commit cfec445
Show file tree
Hide file tree
Showing 7 changed files with 188 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: 1365438)
CCIPHome_supportsInterface:test_supportsInterface() (gas: 9885)
DefensiveExampleTest:test_HappyPath() (gas: 200521)
DefensiveExampleTest:test_Recovery() (gas: 425013)
E2E:test_E2E_3MessagesMMultiOffRampSuccess_gas() (gas: 1487929)
E2E:test_E2E_3MessagesMMultiOffRampSuccess_gas() (gas: 1488604)
ERC165CheckerReverting_supportsInterfaceReverting:test__supportsInterfaceReverting() (gas: 10485)
EtherSenderReceiverTest_ccipReceive:test_ccipReceive_fallbackToWethTransfer() (gas: 96980)
EtherSenderReceiverTest_ccipReceive:test_ccipReceive_happyPath() (gas: 49812)
EtherSenderReceiverTest_ccipReceive:test_ccipReceive_wrongToken() (gas: 17479)
Expand Down Expand Up @@ -195,11 +196,11 @@ NonceManager_applyPreviousRampsUpdates:test_MultipleRampsUpdates() (gas: 123593)
NonceManager_applyPreviousRampsUpdates:test_PreviousRampAlreadySet_overrideAllowed() (gas: 45963)
NonceManager_applyPreviousRampsUpdates:test_SingleRampUpdate() (gas: 66943)
NonceManager_applyPreviousRampsUpdates:test_ZeroInput() (gas: 12191)
NonceManager_getInboundNonce:test_getInboundNonce_NoPrevOffRampForChain() (gas: 178734)
NonceManager_getInboundNonce:test_getInboundNonce_Upgraded() (gas: 145889)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceNewSenderStartsAtZero() (gas: 182209)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceStartsAtV1Nonce() (gas: 244710)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedOffRampNonceSkipsIfMsgInFlight() (gas: 213585)
NonceManager_getInboundNonce:test_getInboundNonce_NoPrevOffRampForChain() (gas: 178809)
NonceManager_getInboundNonce:test_getInboundNonce_Upgraded() (gas: 145964)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceNewSenderStartsAtZero() (gas: 182284)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceStartsAtV1Nonce() (gas: 244860)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedOffRampNonceSkipsIfMsgInFlight() (gas: 213660)
NonceManager_getInboundNonce:test_getInboundNonce_UpgradedSenderNoncesReadsPreviousRamp() (gas: 60520)
NonceManager_getIncrementedOutboundNonce:test_getIncrementedOutboundNonce() (gas: 37979)
NonceManager_getIncrementedOutboundNonce:test_incrementInboundNonce() (gas: 38756)
Expand All @@ -215,12 +216,12 @@ OffRamp_applySourceChainConfigUpdates:test_ApplyZeroUpdates() (gas: 16720)
OffRamp_applySourceChainConfigUpdates:test_ReplaceExistingChain() (gas: 181035)
OffRamp_applySourceChainConfigUpdates:test_ReplaceExistingChainOnRamp() (gas: 168558)
OffRamp_applySourceChainConfigUpdates:test_allowNonOnRampUpdateAfterLaneIsUsed() (gas: 284716)
OffRamp_batchExecute:test_MultipleReportsDifferentChains() (gas: 325065)
OffRamp_batchExecute:test_MultipleReportsDifferentChainsSkipCursedChain() (gas: 170394)
OffRamp_batchExecute:test_MultipleReportsSameChain() (gas: 268364)
OffRamp_batchExecute:test_MultipleReportsSkipDuplicate() (gas: 161411)
OffRamp_batchExecute:test_SingleReport() (gas: 149342)
OffRamp_batchExecute:test_Unhealthy() (gas: 532022)
OffRamp_batchExecute:test_MultipleReportsDifferentChains() (gas: 325290)
OffRamp_batchExecute:test_MultipleReportsDifferentChainsSkipCursedChain() (gas: 170469)
OffRamp_batchExecute:test_MultipleReportsSameChain() (gas: 268589)
OffRamp_batchExecute:test_MultipleReportsSkipDuplicate() (gas: 161486)
OffRamp_batchExecute:test_SingleReport() (gas: 149417)
OffRamp_batchExecute:test_Unhealthy() (gas: 532472)
OffRamp_commit:test_OnlyGasPriceUpdates() (gas: 112719)
OffRamp_commit:test_OnlyTokenPriceUpdates() (gas: 112673)
OffRamp_commit:test_PriceSequenceNumberCleared() (gas: 354787)
Expand All @@ -229,51 +230,51 @@ OffRamp_commit:test_ReportOnlyRootSuccess_gas() (gas: 140878)
OffRamp_commit:test_RootWithRMNDisabled() (gas: 153674)
OffRamp_commit:test_StaleReportWithRoot() (gas: 231732)
OffRamp_commit:test_ValidPriceUpdateThenStaleReportWithRoot() (gas: 206199)
OffRamp_constructor:test_Constructor() (gas: 6210275)
OffRamp_execute:test_LargeBatch() (gas: 3354418)
OffRamp_execute:test_MultipleReports() (gas: 290585)
OffRamp_execute:test_MultipleReportsWithPartialValidationFailures() (gas: 364141)
OffRamp_execute:test_SingleReport() (gas: 168585)
OffRamp_executeSingleMessage:test_executeSingleMessage_NoTokens() (gas: 51593)
OffRamp_constructor:test_Constructor() (gas: 6214884)
OffRamp_execute:test_LargeBatch() (gas: 3356668)
OffRamp_execute:test_MultipleReports() (gas: 290810)
OffRamp_execute:test_MultipleReportsWithPartialValidationFailures() (gas: 364216)
OffRamp_execute:test_SingleReport() (gas: 168660)
OffRamp_executeSingleMessage:test_executeSingleMessage_NoTokens() (gas: 51668)
OffRamp_executeSingleMessage:test_executeSingleMessage_NonContract() (gas: 20508)
OffRamp_executeSingleMessage:test_executeSingleMessage_NonContractWithTokens() (gas: 230350)
OffRamp_executeSingleMessage:test_executeSingleMessage_WithMessageInterceptor() (gas: 87432)
OffRamp_executeSingleMessage:test_executeSingleMessage_WithTokens() (gas: 259856)
OffRamp_executeSingleReport:test_InvalidSourcePoolAddress() (gas: 454636)
OffRamp_executeSingleReport:test_ReceiverError() (gas: 180665)
OffRamp_executeSingleReport:test_SingleMessageNoTokens() (gas: 204824)
OffRamp_executeSingleReport:test_SingleMessageNoTokensOtherChain() (gas: 241135)
OffRamp_executeSingleReport:test_SingleMessageNoTokensUnordered() (gas: 184833)
OffRamp_executeSingleReport:test_SingleMessageToNonCCIPReceiver() (gas: 243821)
OffRamp_executeSingleReport:test_SingleMessagesNoTokensSuccess_gas() (gas: 134435)
OffRamp_executeSingleMessage:test_executeSingleMessage_NonContractWithTokens() (gas: 230500)
OffRamp_executeSingleMessage:test_executeSingleMessage_WithMessageInterceptor() (gas: 87507)
OffRamp_executeSingleMessage:test_executeSingleMessage_WithTokens() (gas: 260081)
OffRamp_executeSingleReport:test_InvalidSourcePoolAddress() (gas: 454936)
OffRamp_executeSingleReport:test_ReceiverError() (gas: 180740)
OffRamp_executeSingleReport:test_SingleMessageNoTokens() (gas: 204974)
OffRamp_executeSingleReport:test_SingleMessageNoTokensOtherChain() (gas: 241285)
OffRamp_executeSingleReport:test_SingleMessageNoTokensUnordered() (gas: 184983)
OffRamp_executeSingleReport:test_SingleMessageToNonCCIPReceiver() (gas: 243852)
OffRamp_executeSingleReport:test_SingleMessagesNoTokensSuccess_gas() (gas: 134510)
OffRamp_executeSingleReport:test_SkippedIncorrectNonce() (gas: 58298)
OffRamp_executeSingleReport:test_SkippedIncorrectNonceStillExecutes() (gas: 391797)
OffRamp_executeSingleReport:test_TwoMessagesWithTokensAndGE() (gas: 561276)
OffRamp_executeSingleReport:test_TwoMessagesWithTokensSuccess_gas() (gas: 509640)
OffRamp_executeSingleReport:test_Unhealthy() (gas: 527805)
OffRamp_executeSingleReport:test_WithCurseOnAnotherSourceChain() (gas: 438850)
OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessage() (gas: 157823)
OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessageUnordered() (gas: 128214)
OffRamp_executeSingleReport:test_SkippedIncorrectNonceStillExecutes() (gas: 392022)
OffRamp_executeSingleReport:test_TwoMessagesWithTokensAndGE() (gas: 561726)
OffRamp_executeSingleReport:test_TwoMessagesWithTokensSuccess_gas() (gas: 510090)
OffRamp_executeSingleReport:test_Unhealthy() (gas: 528255)
OffRamp_executeSingleReport:test_WithCurseOnAnotherSourceChain() (gas: 439300)
OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessage() (gas: 157898)
OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessageUnordered() (gas: 128289)
OffRamp_getExecutionState:test_FillExecutionState() (gas: 3905742)
OffRamp_getExecutionState:test_GetDifferentChainExecutionState() (gas: 121049)
OffRamp_getExecutionState:test_GetExecutionState() (gas: 89738)
OffRamp_manuallyExecute:test_manuallyExecute() (gas: 211986)
OffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched() (gas: 165552)
OffRamp_manuallyExecute:test_manuallyExecute_LowGasLimit() (gas: 475495)
OffRamp_manuallyExecute:test_manuallyExecute_ReentrancyFails() (gas: 2214817)
OffRamp_manuallyExecute:test_manuallyExecute_WithGasOverride() (gas: 212536)
OffRamp_manuallyExecute:test_manuallyExecute_WithMultiReportGasOverride() (gas: 729871)
OffRamp_manuallyExecute:test_manuallyExecute_WithPartialMessages() (gas: 335954)
OffRamp_releaseOrMintSingleToken:test__releaseOrMintSingleToken() (gas: 94597)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens() (gas: 161092)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_WithGasOverride() (gas: 162962)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_destDenominatedDecimals() (gas: 174201)
OffRamp_manuallyExecute:test_manuallyExecute() (gas: 212136)
OffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched() (gas: 165627)
OffRamp_manuallyExecute:test_manuallyExecute_LowGasLimit() (gas: 475645)
OffRamp_manuallyExecute:test_manuallyExecute_ReentrancyFails() (gas: 2214967)
OffRamp_manuallyExecute:test_manuallyExecute_WithGasOverride() (gas: 212686)
OffRamp_manuallyExecute:test_manuallyExecute_WithMultiReportGasOverride() (gas: 730621)
OffRamp_manuallyExecute:test_manuallyExecute_WithPartialMessages() (gas: 336254)
OffRamp_releaseOrMintSingleToken:test__releaseOrMintSingleToken() (gas: 94672)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens() (gas: 161242)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_WithGasOverride() (gas: 163112)
OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_destDenominatedDecimals() (gas: 174351)
OffRamp_setDynamicConfig:test_SetDynamicConfig() (gas: 25397)
OffRamp_setDynamicConfig:test_SetDynamicConfigWithInterceptor() (gas: 47448)
OffRamp_trialExecute:test_trialExecute() (gas: 263492)
OffRamp_trialExecute:test_trialExecute_RateLimitError() (gas: 120655)
OffRamp_trialExecute:test_trialExecute_TokenHandlingErrorIsCaught() (gas: 131965)
OffRamp_trialExecute:test_trialExecute_TokenPoolIsNotAContract() (gas: 281133)
OffRamp_trialExecute:test_trialExecute() (gas: 263717)
OffRamp_trialExecute:test_trialExecute_RateLimitError() (gas: 120730)
OffRamp_trialExecute:test_trialExecute_TokenHandlingErrorIsCaught() (gas: 132040)
OffRamp_trialExecute:test_trialExecute_TokenPoolIsNotAContract() (gas: 281358)
OnRampTokenPoolReentrancy:test_OnRampTokenPoolReentrancy() (gas: 244196)
OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates() (gas: 325984)
OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates_InvalidAllowListRequestDisabledAllowListWithAdds() (gas: 17227)
Expand Down
74 changes: 74 additions & 0 deletions contracts/src/v0.8/ccip/libraries/ERC165CheckerReverting.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// 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 the {IERC165} interface.
/// @dev Any contract that implements ERC165 must explicitly indicate support of InterfaceId_ERC165 and explicitly
/// indicate non-support of InterfaceId_Invalid as per the standard.
/// @param account the address to be queried for support for ERC165
/// @return true if the contract at account indicates support for ERC165, false otherwise
function _supportsERC165Reverting(
address account
) internal view returns (bool) {
return _supportsERC165InterfaceUncheckedReverting(account, type(IERC165).interfaceId)
&& !_supportsERC165InterfaceUncheckedReverting(account, INTERFACE_ID_INVALID);
}

/// @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) {
// query support of both ERC165 as per the spec and support of _interfaceId
return _supportsERC165Reverting(account) && _supportsERC165InterfaceUncheckedReverting(account, 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, <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);
}
}
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 cfec445

Please sign in to comment.