diff --git a/audit/nethermind-lz-cross-chain-rate-limiting.md b/audit/nethermind-lz-cross-chain-rate-limiting.md new file mode 100644 index 00000000..e880ed37 --- /dev/null +++ b/audit/nethermind-lz-cross-chain-rate-limiting.md @@ -0,0 +1,15 @@ +# [NM-0217] Veda vault’s cross-chain x PairwiseRateLimiter + +**File(s)**: [LayerZeroTeller.sol](https://github.com/Se7en-Seas/boring-vault/blob/730929c7410b75a40547a8cc71104b1748c7e578/src/base/Roles/CrossChain/Bridges/LayerZero/LayerZeroTeller.sol), [PairwiseRateLimiter.sol](https://github.com/Se7en-Seas/boring-vault/blob/730929c7410b75a40547a8cc71104b1748c7e578/src/base/Roles/CrossChain/PairwiseRateLimiter.sol) + +### Summary + +The purpose of this PR is to add the rate limiting feature to the Veda vault’s cross-chain bridge. It uses the `PairwiseRateLimiter` contract that was audited previously during the "OFT Security Upgrades" audit item. + +--- + +### Findings + +After reviewing the updated code, we don't see any clear risk on the changes that were implemented. The code seems to work as expected. + +--- diff --git a/src/base/Roles/CrossChain/Bridges/LayerZero/LayerZeroTeller.sol b/src/base/Roles/CrossChain/Bridges/LayerZero/LayerZeroTeller.sol index fa302af7..ff477081 100644 --- a/src/base/Roles/CrossChain/Bridges/LayerZero/LayerZeroTeller.sol +++ b/src/base/Roles/CrossChain/Bridges/LayerZero/LayerZeroTeller.sol @@ -8,12 +8,15 @@ import {SafeTransferLib} from "@solmate/utils/SafeTransferLib.sol"; import {OAppAuth, Origin, MessagingFee, MessagingReceipt} from "@oapp-auth/OAppAuth.sol"; import {AddressToBytes32Lib} from "src/helper/AddressToBytes32Lib.sol"; import {OptionsBuilder} from "@oapp-auth/OptionsBuilder.sol"; +import {PairwiseRateLimiter} from "src/base/Roles/CrossChain/PairwiseRateLimiter.sol"; +import {MessageLib} from "src/base/Roles/CrossChain/MessageLib.sol"; -contract LayerZeroTeller is CrossChainTellerWithGenericBridge, OAppAuth { +contract LayerZeroTeller is CrossChainTellerWithGenericBridge, OAppAuth, PairwiseRateLimiter { using SafeTransferLib for ERC20; using AddressToBytes32Lib for address; using AddressToBytes32Lib for bytes32; using OptionsBuilder for bytes; + using MessageLib for uint256; // ========================================= STRUCTS ========================================= @@ -164,6 +167,22 @@ contract LayerZeroTeller is CrossChainTellerWithGenericBridge, OAppAuth { emit ChainStopMessagesTo(chainId); } + /** + * @notice Set outbound rate limit configurations. + * @dev Callable by MULTISIG_ROLE. + */ + function setOutboundRateLimits(RateLimitConfig[] calldata _rateLimitConfigs) external requiresAuth { + _setOutboundRateLimits(_rateLimitConfigs); + } + + /** + * @notice Set inbound rate limit configurations. + * @dev Callable by MULTISIG_ROLE. + */ + function setInboundRateLimits(RateLimitConfig[] calldata _rateLimitConfigs) external requiresAuth { + _setInboundRateLimits(_rateLimitConfigs); + } + /** * @notice Set the gas limit for messages to a chain. * @dev Callable by OWNER_ROLE. @@ -195,6 +214,7 @@ contract LayerZeroTeller is CrossChainTellerWithGenericBridge, OAppAuth { Chain memory source = idToChains[_origin.srcEid]; if (!source.allowMessagesFrom) revert LayerZeroTeller__MessagesNotAllowedFrom(_origin.srcEid); uint256 message = abi.decode(_message, (uint256)); + _checkAndUpdateInboundRateLimit(_origin.srcEid, message.uint256ToMessage().shareAmount); _completeMessageReceive(_guid, message); } @@ -217,6 +237,7 @@ contract LayerZeroTeller is CrossChainTellerWithGenericBridge, OAppAuth { returns (bytes32 messageId) { uint32 destinationId = abi.decode(bridgeWildCard, (uint32)); + _checkAndUpdateOutboundRateLimit(destinationId, message.uint256ToMessage().shareAmount); Chain memory chain = idToChains[destinationId]; if (!chain.allowMessagesTo) { revert LayerZeroTeller__MessagesNotAllowedTo(destinationId); diff --git a/src/base/Roles/CrossChain/PairwiseRateLimiter.sol b/src/base/Roles/CrossChain/PairwiseRateLimiter.sol new file mode 100644 index 00000000..8a10159e --- /dev/null +++ b/src/base/Roles/CrossChain/PairwiseRateLimiter.sol @@ -0,0 +1,206 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +/** + * @title Rate Limiter + * @dev Extends LayerZero's evm-oapp v2 RateLimiter contract. The original contract only supports rate limiting for outbound messages. + * This contract adds support for rate limiting inbound messages. + */ + +abstract contract PairwiseRateLimiter { + /** + * @notice Rate Limit struct. + * @param amountInFlight The amount in the current window. + * @param lastUpdated Timestamp representing the last time the rate limit was checked or updated. + * @param limit This represents the maximum allowed amount within a given window. + * @param window Defines the duration of the rate limiting window. + */ + struct RateLimit { + uint256 amountInFlight; + uint256 lastUpdated; + uint256 limit; + uint256 window; + } + + /** + * @notice Rate Limit Configuration struct. + * @param dstEid The peer endpoint id. + * @param limit This represents the maximum allowed amount within a given window. + * @param window Defines the duration of the rate limiting window. + */ + struct RateLimitConfig { + uint32 peerEid; + uint256 limit; + uint256 window; + } + + /** + * @dev Mapping from peer endpoint id to RateLimit Configurations. + */ + mapping(uint32 dstEid => RateLimit limit) public outboundRateLimits; + mapping(uint32 srcEid => RateLimit limit) public inboundRateLimits; + + /** + * @notice Emitted when _setRateLimits occurs. + * @param rateLimitConfigs An array of `RateLimitConfig` structs representing the rate limit configurations set. + * - `peerEid`: The peer endpoint id. + * - `limit`: This represents the maximum allowed amount within a given window. + * - `window`: Defines the duration of the rate limiting window. + */ + event OutboundRateLimitsChanged(RateLimitConfig[] rateLimitConfigs); + event InboundRateLimitsChanged(RateLimitConfig[] rateLimitConfigs); + + /** + * @notice Error that is thrown when an amount exceeds the rate_limit. + */ + error OutboundRateLimitExceeded(); + error InboundRateLimitExceeded() ; + + /** + * @notice Get the current amount that can be sent to this peer endpoint id for the given rate limit window. + * @param _dstEid The destination endpoint id. + * @return outboundAmountInFlight The current amount that was sent. + * @return amountCanBeSent The amount that can be sent. + */ + function getAmountCanBeSent( + uint32 _dstEid + ) external view virtual returns (uint256 outboundAmountInFlight, uint256 amountCanBeSent) { + RateLimit memory rl = outboundRateLimits[_dstEid]; + return _amountCanBeSent(rl.amountInFlight, rl.lastUpdated, rl.limit, rl.window); + } + + /** + * @notice Get the current amount that can received from this peer endpoint for the given rate limit window. + * @param _srcEid The source endpoint id. + * @return inboundAmountInFlight The current amount has been received. + * @return amountCanBeReceived The amount that be received. + */ + function getAmountCanBeReceived( + uint32 _srcEid + ) external view virtual returns (uint256 inboundAmountInFlight, uint256 amountCanBeReceived) { + RateLimit memory rl = inboundRateLimits[_srcEid]; + return _amountCanBeSent(rl.amountInFlight, rl.lastUpdated, rl.limit, rl.window); + } + + /** + * @notice Sets the Rate Limit. + * @param _rateLimitConfigs A `RateLimitConfig` struct representing the rate limit configuration. + * - `dstEid`: The destination endpoint id. + * - `limit`: This represents the maximum allowed amount within a given window. + * - `window`: Defines the duration of the rate limiting window. + */ + function _setOutboundRateLimits(RateLimitConfig[] memory _rateLimitConfigs) internal virtual { + unchecked { + for (uint256 i = 0; i < _rateLimitConfigs.length; i++) { + RateLimit storage rl = outboundRateLimits[_rateLimitConfigs[i].peerEid]; + + // @dev Ensure we checkpoint the existing rate limit as to not retroactively apply the new decay rate. + _checkAndUpdateOutboundRateLimit(_rateLimitConfigs[i].peerEid, 0); + + // @dev Does NOT reset the amountInFlight/lastUpdated of an existing rate limit. + rl.limit = _rateLimitConfigs[i].limit; + rl.window = _rateLimitConfigs[i].window; + } + } + emit OutboundRateLimitsChanged(_rateLimitConfigs); + } + + /** + * @notice Sets the Rate Limit. + * @param _rateLimitConfigs A `RateLimitConfig` struct representing the rate limit configuration. + * - `srcEid`: The source endpoint id. + * - `limit`: This represents the maximum allowed amount within a given window. + * - `window`: Defines the duration of the rate limiting window. + */ + function _setInboundRateLimits(RateLimitConfig[] memory _rateLimitConfigs) internal virtual { + unchecked { + for (uint256 i = 0; i < _rateLimitConfigs.length; i++) { + RateLimit storage rl = inboundRateLimits[_rateLimitConfigs[i].peerEid]; + + // @dev Ensure we checkpoint the existing rate limit as to not retroactively apply the new decay rate. + _checkAndUpdateInboundRateLimit(_rateLimitConfigs[i].peerEid, 0); + + // @dev Does NOT reset the amountInFlight/lastUpdated of an existing rate limit. + rl.limit = _rateLimitConfigs[i].limit; + rl.window = _rateLimitConfigs[i].window; + } + } + emit InboundRateLimitsChanged(_rateLimitConfigs); + } + + /** + * @notice Checks current amount in flight and amount that can be sent for a given rate limit window. + * @param _amountInFlight The amount in the current window. + * @param _lastUpdated Timestamp representing the last time the rate limit was checked or updated. + * @param _limit This represents the maximum allowed amount within a given window. + * @param _window Defines the duration of the rate limiting window. + * @return currentAmountInFlight The amount in the current window. + * @return amountCanBeSent The amount that can be sent. + */ + function _amountCanBeSent( + uint256 _amountInFlight, + uint256 _lastUpdated, + uint256 _limit, + uint256 _window + ) internal view virtual returns (uint256 currentAmountInFlight, uint256 amountCanBeSent) { + uint256 timeSinceLastDeposit = block.timestamp - _lastUpdated; + if (timeSinceLastDeposit >= _window) { + currentAmountInFlight = 0; + amountCanBeSent = _limit; + } else { + // @dev Presumes linear decay. + uint256 decay = (_limit * timeSinceLastDeposit) / _window; + currentAmountInFlight = _amountInFlight <= decay ? 0 : _amountInFlight - decay; + // @dev In the event the _limit is lowered, and the 'in-flight' amount is higher than the _limit, set to 0. + amountCanBeSent = _limit <= currentAmountInFlight ? 0 : _limit - currentAmountInFlight; + } + } + + /** + * @notice Verifies whether the specified amount falls within the rate limit constraints for the targeted + * endpoint ID. On successful verification, it updates amountInFlight and lastUpdated. If the amount exceeds + * the rate limit, the operation reverts. + * @param _dstEid The destination endpoint id. + * @param _amount The amount to check for rate limit constraints. + */ + function _checkAndUpdateOutboundRateLimit(uint32 _dstEid, uint256 _amount) internal virtual { + // @dev By default dstEid that have not been explicitly set will return amountCanBeSent == 0. + RateLimit storage rl = outboundRateLimits[_dstEid]; + + (uint256 currentAmountInFlight, uint256 amountCanBeSent) = _amountCanBeSent( + rl.amountInFlight, + rl.lastUpdated, + rl.limit, + rl.window + ); + if (_amount > amountCanBeSent) revert OutboundRateLimitExceeded(); + + // @dev Update the storage to contain the new amount and current timestamp. + rl.amountInFlight = currentAmountInFlight + _amount; + rl.lastUpdated = block.timestamp; + } + + /** + * @notice Verifies whether the specified amount falls within the rate limit constraints for the targeted + * endpoint ID. On successful verification, it updates amountInFlight and lastUpdated. If the amount exceeds + * the rate limit, the operation reverts. + * @param _srcEid The source endpoint id. + * @param _amount The amount to check for rate limit constraints. + */ + function _checkAndUpdateInboundRateLimit(uint32 _srcEid, uint256 _amount) internal virtual { + // @dev By default dstEid that have not been explicitly set will return amountCanBeSent == 0. + RateLimit storage rl = inboundRateLimits[_srcEid]; + + (uint256 currentAmountInFlight, uint256 amountCanBeSent) = _amountCanBeSent( + rl.amountInFlight, + rl.lastUpdated, + rl.limit, + rl.window + ); + if (_amount > amountCanBeSent) revert InboundRateLimitExceeded(); + + // @dev Update the storage to contain the new amount and current timestamp. + rl.amountInFlight = currentAmountInFlight + _amount; + rl.lastUpdated = block.timestamp; + } +} diff --git a/test/LayerZeroTeller.t.sol b/test/LayerZeroTeller.t.sol index f37ec25d..07b6f0d8 100644 --- a/test/LayerZeroTeller.t.sol +++ b/test/LayerZeroTeller.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.21; import {BoringVault} from "src/base/BoringVault.sol"; import {LayerZeroTeller} from "src/base/Roles/CrossChain/Bridges/LayerZero/LayerZeroTeller.sol"; +import {PairwiseRateLimiter} from "src/base/Roles/CrossChain/PairwiseRateLimiter.sol"; import {AccountantWithRateProviders} from "src/base/Roles/AccountantWithRateProviders.sol"; import {SafeTransferLib} from "@solmate/utils/SafeTransferLib.sol"; import {FixedPointMathLib} from "@solmate/utils/FixedPointMathLib.sol"; @@ -133,6 +134,10 @@ contract LayerZeroTellerTest is Test, MerkleTreeHelper { // Setup chains on bridge. sourceTeller.addChain(DESTINATION_ID, true, true, address(destinationTeller), 1_000_000); destinationTeller.addChain(SOURCE_ID, true, true, address(sourceTeller), 1_000_000); + + // Setup rate limiting. + sourceTeller.setOutboundRateLimits(createRateLimitConfig(DESTINATION_ID, 2000 ether, 4 hours)); + destinationTeller.setInboundRateLimits(createRateLimitConfig(SOURCE_ID, 2000 ether, 4 hours)); } function testBridgingShares(uint96 sharesToBridge) external { @@ -216,6 +221,35 @@ contract LayerZeroTellerTest is Test, MerkleTreeHelper { } function testReverts() external { + uint96 sharesToBridge = uint96(bound(uint96(101), 1, 1_000e18)); + bytes memory bridgeData = abi.encode(DESTINATION_ID); + uint256 bridgeValue = 0.001e18; + address to = vm.addr(1); + + // Test outbound rate limit. + sourceTeller.setOutboundRateLimits(createRateLimitConfig(DESTINATION_ID, 100, 4 hours)); + // Expect failure by exceeding limit. + vm.expectRevert(PairwiseRateLimiter.OutboundRateLimitExceeded.selector); + sourceTeller.bridge{value: bridgeValue}( sharesToBridge, to, bridgeData, NATIVE_ERC20, 1e18); + + // Increase limit and retry + sourceTeller.setOutboundRateLimits(createRateLimitConfig(DESTINATION_ID, 2000 ether, 4 hours)); + sourceTeller.bridge{value: bridgeValue}( sharesToBridge, to, bridgeData, NATIVE_ERC20, 1e18); + + // Test inbound rate limit. + destinationTeller.setInboundRateLimits(createRateLimitConfig(SOURCE_ID, 100, 4 hours)); + MockLayerZeroEndPoint.Packet memory m = endPoint.getLastMessage(); + // Expect failure by exceeding limit. + vm.prank(address(endPoint)); + vm.expectRevert(PairwiseRateLimiter.InboundRateLimitExceeded.selector); + LayerZeroTeller(m.to).lzReceive(m._origin, m._guid, m._message, m._executor, m._extraData); + + // Reset limit and retry. + destinationTeller.setInboundRateLimits(createRateLimitConfig(SOURCE_ID, 2000 ether, 4 hours)); + vm.prank(address(endPoint)); + LayerZeroTeller(m.to).lzReceive(m._origin, m._guid, m._message, m._executor, m._extraData); + assertEq(boringVault.balanceOf(to), sharesToBridge, "To address should have received shares."); + // Adding a chain with a zero message gas limit should revert. vm.expectRevert(bytes(abi.encodeWithSelector(LayerZeroTeller.LayerZeroTeller__ZeroMessageGasLimit.selector))); sourceTeller.addChain(DESTINATION_ID, true, true, address(destinationTeller), 0); @@ -268,7 +302,7 @@ contract LayerZeroTellerTest is Test, MerkleTreeHelper { sourceTeller.bridge{value: 0.001e18}(1e18, address(this), abi.encode(DESTINATION_ID), NATIVE_ERC20, 1e18); - MockLayerZeroEndPoint.Packet memory m = endPoint.getLastMessage(); + m = endPoint.getLastMessage(); // Send message to destination. vm.startPrank(address(endPoint)); @@ -302,4 +336,18 @@ contract LayerZeroTellerTest is Test, MerkleTreeHelper { forkId = vm.createFork(vm.envString(rpcKey), blockNumber); vm.selectFork(forkId); } + + function createRateLimitConfig( + uint32 peerId, + uint256 limit, + uint256 window + ) internal pure returns (PairwiseRateLimiter.RateLimitConfig[] memory) { + PairwiseRateLimiter.RateLimitConfig[] memory configs = new PairwiseRateLimiter.RateLimitConfig[](1); + configs[0] = PairwiseRateLimiter.RateLimitConfig({ + peerEid: peerId, + limit: limit, + window: window + }); + return configs; + } }