Skip to content

Commit

Permalink
CCIP-3789 add check for abi-encoded zero-address (#14960)
Browse files Browse the repository at this point in the history 
* CCIP-3789 add check for abi-encoded zero-address

* Update gethwrappers

* gas snapshot fix

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
  • Loading branch information
@jhweintraub @app-token-issuer-infra-releng
jhweintraub and app-token-issuer-infra-releng[bot] authored Oct 28, 2024
1 parent f3b7fe0 commit 1b1dc3b
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 14 deletions.
5 changes: 5 additions & 0 deletions contracts/.changeset/tasty-eggs-yell.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@chainlink/contracts': patch
---

CCIP-3789 Add check on MultiAggregateRateLimiter:UpdateRateLimitTokens that remote token is not abi.encode(address(0)) #bugfix
23 changes: 12 additions & 11 deletions contracts/gas-snapshots/ccip.gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,42 +300,43 @@ MultiAggregateRateLimiter_applyRateLimiterConfigUpdates:test_UpdateExistingConfi
MultiAggregateRateLimiter_applyRateLimiterConfigUpdates:test_UpdateExistingConfig_Success() (gas: 53937)
MultiAggregateRateLimiter_applyRateLimiterConfigUpdates:test_ZeroChainSelector_Revert() (gas: 17154)
MultiAggregateRateLimiter_applyRateLimiterConfigUpdates:test_ZeroConfigs_Success() (gas: 12481)
MultiAggregateRateLimiter_constructor:test_ConstructorNoAuthorizedCallers_Success() (gas: 1995557)
MultiAggregateRateLimiter_constructor:test_Constructor_Success() (gas: 2111854)
MultiAggregateRateLimiter_constructor:test_ConstructorNoAuthorizedCallers_Success() (gas: 2005579)
MultiAggregateRateLimiter_constructor:test_Constructor_Success() (gas: 2121876)
MultiAggregateRateLimiter_getTokenBucket:test_GetTokenBucket_Success() (gas: 30794)
MultiAggregateRateLimiter_getTokenBucket:test_Refill_Success() (gas: 48169)
MultiAggregateRateLimiter_getTokenBucket:test_TimeUnderflow_Revert() (gas: 15907)
MultiAggregateRateLimiter_getTokenValue:test_GetTokenValue_Success() (gas: 17602)
MultiAggregateRateLimiter_getTokenValue:test_NoTokenPrice_Reverts() (gas: 21630)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageFromUnauthorizedCaller_Revert() (gas: 14647)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageWithDifferentTokensOnDifferentChains_Success() (gas: 210393)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageWithDifferentTokensOnDifferentChains_Success() (gas: 210589)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageWithDisabledRateLimitToken_Success() (gas: 58469)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageWithNoTokens_Success() (gas: 17809)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageWithRateLimitDisabled_Success() (gas: 45220)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageWithRateLimitExceeded_Revert() (gas: 46488)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageWithRateLimitReset_Success() (gas: 76929)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageWithTokensOnDifferentChains_Success() (gas: 308577)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageWithTokensOnDifferentChains_Success() (gas: 308969)
MultiAggregateRateLimiter_onInboundMessage:test_ValidateMessageWithTokens_Success() (gas: 50654)
MultiAggregateRateLimiter_onOutboundMessage:test_RateLimitValueDifferentLanes_Success() (gas: 51305)
MultiAggregateRateLimiter_onOutboundMessage:test_ValidateMessageWithNoTokens_Success() (gas: 19393)
MultiAggregateRateLimiter_onOutboundMessage:test_onOutboundMessage_ValidateMessageFromUnauthorizedCaller_Revert() (gas: 15925)
MultiAggregateRateLimiter_onOutboundMessage:test_onOutboundMessage_ValidateMessageWithDifferentTokensOnDifferentChains_Success() (gas: 210113)
MultiAggregateRateLimiter_onOutboundMessage:test_onOutboundMessage_ValidateMessageWithDifferentTokensOnDifferentChains_Success() (gas: 210309)
MultiAggregateRateLimiter_onOutboundMessage:test_onOutboundMessage_ValidateMessageWithDisabledRateLimitToken_Success() (gas: 60262)
MultiAggregateRateLimiter_onOutboundMessage:test_onOutboundMessage_ValidateMessageWithRateLimitDisabled_Success() (gas: 47043)
MultiAggregateRateLimiter_onOutboundMessage:test_onOutboundMessage_ValidateMessageWithRateLimitExceeded_Revert() (gas: 48279)
MultiAggregateRateLimiter_onOutboundMessage:test_onOutboundMessage_ValidateMessageWithRateLimitReset_Success() (gas: 77936)
MultiAggregateRateLimiter_onOutboundMessage:test_onOutboundMessage_ValidateMessageWithTokensOnDifferentChains_Success() (gas: 308523)
MultiAggregateRateLimiter_onOutboundMessage:test_onOutboundMessage_ValidateMessageWithTokensOnDifferentChains_Success() (gas: 308915)
MultiAggregateRateLimiter_onOutboundMessage:test_onOutboundMessage_ValidateMessageWithTokens_Success() (gas: 52424)
MultiAggregateRateLimiter_setFeeQuoter:test_OnlyOwner_Revert() (gas: 11381)
MultiAggregateRateLimiter_setFeeQuoter:test_Owner_Success() (gas: 19190)
MultiAggregateRateLimiter_setFeeQuoter:test_ZeroAddress_Revert() (gas: 10642)
MultiAggregateRateLimiter_updateRateLimitTokens:test_NonOwner_Revert() (gas: 18926)
MultiAggregateRateLimiter_updateRateLimitTokens:test_UpdateRateLimitTokensMultipleChains_Success() (gas: 280586)
MultiAggregateRateLimiter_updateRateLimitTokens:test_UpdateRateLimitTokensSingleChain_Success() (gas: 254999)
MultiAggregateRateLimiter_updateRateLimitTokens:test_UpdateRateLimitTokens_AddsAndRemoves_Success() (gas: 204837)
MultiAggregateRateLimiter_updateRateLimitTokens:test_UpdateRateLimitTokensMultipleChains_Success() (gas: 281000)
MultiAggregateRateLimiter_updateRateLimitTokens:test_UpdateRateLimitTokensSingleChain_Success() (gas: 255391)
MultiAggregateRateLimiter_updateRateLimitTokens:test_UpdateRateLimitTokens_AddsAndRemoves_Success() (gas: 205169)
MultiAggregateRateLimiter_updateRateLimitTokens:test_UpdateRateLimitTokens_RemoveNonExistentToken_Success() (gas: 29012)
MultiAggregateRateLimiter_updateRateLimitTokens:test_ZeroDestToken_Revert() (gas: 18358)
MultiAggregateRateLimiter_updateRateLimitTokens:test_ZeroSourceToken_Revert() (gas: 18277)
MultiAggregateRateLimiter_updateRateLimitTokens:test_ZeroDestToken_AbiEncoded_Revert() (gas: 14001)
MultiAggregateRateLimiter_updateRateLimitTokens:test_ZeroDestToken_Revert() (gas: 18365)
MultiAggregateRateLimiter_updateRateLimitTokens:test_ZeroSourceToken_Revert() (gas: 18294)
MultiOCR3Base_setOCR3Configs:test_FMustBePositive_Revert() (gas: 59441)
MultiOCR3Base_setOCR3Configs:test_FTooHigh_Revert() (gas: 44212)
MultiOCR3Base_setOCR3Configs:test_MoreTransmittersThanSigners_Revert() (gas: 104844)
Expand Down
4 changes: 3 additions & 1 deletion contracts/src/v0.8/ccip/MultiAggregateRateLimiter.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ contract MultiAggregateRateLimiter is IMessageInterceptor, AuthorizedCallers, IT

string public constant override typeAndVersion = "MultiAggregateRateLimiter 1.6.0-dev";

bytes32 private constant EMPTY_ENCODED_ADDRESS_HASH = keccak256(abi.encode(address(0)));

/// @dev Tokens that should be included in Aggregate Rate Limiting (from local chain (this chain) -> remote),
/// grouped per-remote chain.
mapping(uint64 remoteChainSelector => EnumerableMapAddresses.AddressToBytesMap tokensLocalToRemote) private
Expand Down Expand Up @@ -240,7 +242,7 @@ contract MultiAggregateRateLimiter is IMessageInterceptor, AuthorizedCallers, IT
bytes memory remoteToken = adds[i].remoteToken;
address localToken = localTokenArgs.localToken;

if (localToken == address(0) || remoteToken.length == 0) {
if (localToken == address(0) || remoteToken.length == 0 || keccak256(remoteToken) == EMPTY_ENCODED_ADDRESS_HASH) {
revert ZeroAddressNotAllowed();
}

Expand Down
14 changes: 14 additions & 0 deletions contracts/src/v0.8/ccip/test/rateLimiter/MultiAggregateRateLimiter.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,20 @@ contract MultiAggregateRateLimiter_updateRateLimitTokens is MultiAggregateRateLi
s_rateLimiter.updateRateLimitTokens(new MultiAggregateRateLimiter.LocalRateLimitToken[](0), adds);
}

function test_ZeroDestToken_AbiEncoded_Revert() public {
MultiAggregateRateLimiter.RateLimitTokenArgs[] memory adds = new MultiAggregateRateLimiter.RateLimitTokenArgs[](1);
adds[0] = MultiAggregateRateLimiter.RateLimitTokenArgs({
localTokenArgs: MultiAggregateRateLimiter.LocalRateLimitToken({
remoteChainSelector: CHAIN_SELECTOR_1,
localToken: address(0)
}),
remoteToken: abi.encode(address(0))
});

vm.expectRevert(AuthorizedCallers.ZeroAddressNotAllowed.selector);
s_rateLimiter.updateRateLimitTokens(new MultiAggregateRateLimiter.LocalRateLimitToken[](0), adds);
}

function test_NonOwner_Revert() public {
MultiAggregateRateLimiter.RateLimitTokenArgs[] memory adds = new MultiAggregateRateLimiter.RateLimitTokenArgs[](4);

Expand Down
2 changes: 1 addition & 1 deletion .../gethwrappers/ccip/generated/multi_aggregate_rate_limiter/multi_aggregate_rate_limiter.go
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion core/gethwrappers/ccip/generation/generated-wrapper-dependency-versions-do-not-edit.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ message_hasher: ../../../contracts/solc/v0.8.24/MessageHasher/MessageHasher.abi
mock_usdc_token_messenger: ../../../contracts/solc/v0.8.24/MockE2EUSDCTokenMessenger/MockE2EUSDCTokenMessenger.abi ../../../contracts/solc/v0.8.24/MockE2EUSDCTokenMessenger/MockE2EUSDCTokenMessenger.bin d976651d36b33ac2196b32b9d2f4fa6690c6a18d41b621365659fce1c1d1e737
mock_usdc_token_transmitter: ../../../contracts/solc/v0.8.24/MockE2EUSDCTransmitter/MockE2EUSDCTransmitter.abi ../../../contracts/solc/v0.8.24/MockE2EUSDCTransmitter/MockE2EUSDCTransmitter.bin be0dbc3e475741ea0b7a54ec2b935a321b428baa9f4ce18180a87fb38bb87de2
mock_v3_aggregator_contract: ../../../contracts/solc/v0.8.24/MockV3Aggregator/MockV3Aggregator.abi ../../../contracts/solc/v0.8.24/MockV3Aggregator/MockV3Aggregator.bin 518e19efa2ff52b0fefd8e597b05765317ee7638189bfe34ca43de2f6599faf4
multi_aggregate_rate_limiter: ../../../contracts/solc/v0.8.24/MultiAggregateRateLimiter/MultiAggregateRateLimiter.abi ../../../contracts/solc/v0.8.24/MultiAggregateRateLimiter/MultiAggregateRateLimiter.bin cc63ca92012b7473f77a1c49f77ee20f9242bbecaad2c73c1dc581b447a77e91
multi_aggregate_rate_limiter: ../../../contracts/solc/v0.8.24/MultiAggregateRateLimiter/MultiAggregateRateLimiter.abi ../../../contracts/solc/v0.8.24/MultiAggregateRateLimiter/MultiAggregateRateLimiter.bin 55d35e20c49a61849247a693a0decf44daa9e7820919a15ee5b8ef46e55637ca
multi_ocr3_helper: ../../../contracts/solc/v0.8.24/MultiOCR3Helper/MultiOCR3Helper.abi ../../../contracts/solc/v0.8.24/MultiOCR3Helper/MultiOCR3Helper.bin 04b6b261dd71925670bf4d904aaf7bf08543452009feefb88e07d4c49d12e969
nonce_manager: ../../../contracts/solc/v0.8.24/NonceManager/NonceManager.abi ../../../contracts/solc/v0.8.24/NonceManager/NonceManager.bin eb234dfb57c6dc64f83cfbf9d78a27939a7241fd0de41342d41c919c156a3633
offramp: ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.abi ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.bin 4d0597cc9d04a7901911b2bf7891792bbc600efc0620375a4b5bf033926c9186
Expand Down

0 comments on commit 1b1dc3b

Please sign in to comment.