From 113048e97b2ade2452a9ed8c29854d43dd520ae8 Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Mon, 23 Oct 2023 16:39:29 +0200 Subject: [PATCH 1/3] docs(bundlers): add reetrancy disclosure --- src/ERC4626Bundler.sol | 12 ++++++++---- src/Permit2Bundler.sol | 1 + src/PermitBundler.sol | 1 + src/TransferBundler.sol | 3 +++ src/UrdBundler.sol | 1 + src/migration/AaveV2MigrationBundler.sol | 1 + src/migration/AaveV3MigrationBundler.sol | 1 + src/migration/AaveV3OptimizerMigrationBundler.sol | 1 + src/migration/CompoundV2MigrationBundler.sol | 2 ++ src/migration/CompoundV3MigrationBundler.sol | 4 ++++ 10 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/ERC4626Bundler.sol b/src/ERC4626Bundler.sol index 22219e9d..764953ed 100644 --- a/src/ERC4626Bundler.sol +++ b/src/ERC4626Bundler.sol @@ -19,8 +19,9 @@ abstract contract ERC4626Bundler is BaseBundler { /* ACTIONS */ /// @notice Mints the given amount of `shares` on the given ERC4626 `vault`, on behalf of `owner`. - /// @dev Pass `type(uint256).max` as `shares` to mint max. + /// @dev Warning: `vault` can re-enter the bundler flow. /// @dev Assumes the given `vault` implements EIP-4626. + /// @dev Pass `type(uint256).max` as `shares` to mint max. function erc4626Mint(address vault, uint256 shares, address owner) external payable { require(owner != address(0), ErrorsLib.ZERO_ADDRESS); /// Do not check `owner != address(this)` to allow the bundler to receive the vault's shares. @@ -39,8 +40,9 @@ abstract contract ERC4626Bundler is BaseBundler { } /// @notice Deposits the given amount of `assets` on the given ERC4626 `vault`, on behalf of `owner`. - /// @dev Pass `type(uint256).max` as `assets` to deposit max. + /// @dev Warning: `vault` can re-enter the bundler flow. /// @dev Assumes the given `vault` implements EIP-4626. + /// @dev Pass `type(uint256).max` as `assets` to deposit max. function erc4626Deposit(address vault, uint256 assets, address owner) external payable { require(owner != address(0), ErrorsLib.ZERO_ADDRESS); /// Do not check `owner != address(this)` to allow the bundler to receive the vault's shares. @@ -61,8 +63,9 @@ abstract contract ERC4626Bundler is BaseBundler { /// @notice Withdraws the given amount of `assets` from the given ERC4626 `vault`, transferring assets to /// `receiver`. /// @notice Warning: should only be called via the bundler's `multicall` function. - /// @dev Pass `type(uint256).max` as `assets` to withdraw max. + /// @dev Warning: `vault` can re-enter the bundler flow. /// @dev Assumes the given `vault` implements EIP-4626. + /// @dev Pass `type(uint256).max` as `assets` to withdraw max. function erc4626Withdraw(address vault, uint256 assets, address receiver) external payable { require(receiver != address(0), ErrorsLib.ZERO_ADDRESS); /// Do not check `receiver != address(this)` to allow the bundler to receive the underlying asset. @@ -78,8 +81,9 @@ abstract contract ERC4626Bundler is BaseBundler { /// @notice Redeems the given amount of `shares` from the given ERC4626 `vault`, transferring assets to `receiver`. /// @notice Warning: should only be called via the bundler's `multicall` function. - /// @dev Pass `type(uint256).max` as `shares` to redeem max. + /// @dev Warning: `vault` can re-enter the bundler flow. /// @dev Assumes the given `vault` implements EIP-4626. + /// @dev Pass `type(uint256).max` as `shares` to redeem max. function erc4626Redeem(address vault, uint256 shares, address receiver) external payable { require(receiver != address(0), ErrorsLib.ZERO_ADDRESS); /// Do not check `receiver != address(this)` to allow the bundler to receive the underlying asset. diff --git a/src/Permit2Bundler.sol b/src/Permit2Bundler.sol index 7be7d88e..af617559 100644 --- a/src/Permit2Bundler.sol +++ b/src/Permit2Bundler.sol @@ -19,6 +19,7 @@ abstract contract Permit2Bundler is BaseBundler { /// @notice Permits and performs a transfer from the initiator to the recipient via Permit2. /// @notice Warning: should only be called via the bundler's `multicall` function. + /// @dev Warning: `permit.permitted.token` can re-enter the bundler flow. /// @dev Pass `permit.permitted.amount = type(uint256).max` to transfer all. function permit2TransferFrom(ISignatureTransfer.PermitTransferFrom memory permit, bytes memory signature) external diff --git a/src/PermitBundler.sol b/src/PermitBundler.sol index f20ff6da..0e761854 100644 --- a/src/PermitBundler.sol +++ b/src/PermitBundler.sol @@ -13,6 +13,7 @@ abstract contract PermitBundler is BaseBundler { /// @notice Permits the given `amount` of `asset` from sender to be spent by the bundler via EIP-2612 Permit with /// the given `deadline` & EIP-712 signature's `v`, `r` & `s`. /// @notice Warning: should only be called via the bundler's `multicall` function. + /// @dev Warning: `asset` can re-enter the bundler flow. /// @dev Pass `skipRevert = true` to avoid reverting the whole bundle in case the signature expired. function permit(address asset, uint256 amount, uint256 deadline, uint8 v, bytes32 r, bytes32 s, bool skipRevert) external diff --git a/src/TransferBundler.sol b/src/TransferBundler.sol index 4b3e5169..9ff7c837 100644 --- a/src/TransferBundler.sol +++ b/src/TransferBundler.sol @@ -19,6 +19,7 @@ abstract contract TransferBundler is BaseBundler { /// @notice Transfers the minimum between the given `amount` and the bundler's balance of native asset from the /// bundler to `recipient`. + /// @dev Warning: `recipient` can re-enter the bundler flow. /// @dev Pass `amount = type(uint256).max` to transfer all. function nativeTransfer(address recipient, uint256 amount) external payable { require(recipient != address(0), ErrorsLib.ZERO_ADDRESS); @@ -33,6 +34,7 @@ abstract contract TransferBundler is BaseBundler { /// @notice Transfers the minimum between the given `amount` and the bundler's balance of `asset` from the bundler /// to `recipient`. + /// @dev Warning: `asset` can re-enter the bundler flow. /// @dev Pass `amount = type(uint256).max` to transfer all. function erc20Transfer(address asset, address recipient, uint256 amount) external payable { require(recipient != address(0), ErrorsLib.ZERO_ADDRESS); @@ -47,6 +49,7 @@ abstract contract TransferBundler is BaseBundler { /// @notice Transfers the given `amount` of `asset` from sender to this contract via ERC20 transferFrom. /// @notice Warning: should only be called via the bundler's `multicall` function. + /// @dev Warning: `asset` can re-enter the bundler flow. /// @dev Pass `amount = type(uint256).max` to transfer all. function erc20TransferFrom(address asset, uint256 amount) external payable { address initiator = initiator(); diff --git a/src/UrdBundler.sol b/src/UrdBundler.sol index dc8cd165..7ed86fd6 100644 --- a/src/UrdBundler.sol +++ b/src/UrdBundler.sol @@ -14,6 +14,7 @@ import {BaseBundler} from "./BaseBundler.sol"; /// @notice Bundler that allows to claim token rewards on the Universal Rewards Distributor. abstract contract UrdBundler is BaseBundler { /// @notice Claims `amount` of `reward` on behalf of `account` on the given rewards distributor, using `proof`. + /// @dev Warning: `distributor` can re-enter the bundler flow. /// @dev Assumes the given distributor implements IUniversalRewardsDistributor. /// @dev Pass `skipRevert = true` to avoid reverting the whole bundle in case the proof expired. function urdClaim( diff --git a/src/migration/AaveV2MigrationBundler.sol b/src/migration/AaveV2MigrationBundler.sol index 52f323c3..5a025ee8 100644 --- a/src/migration/AaveV2MigrationBundler.sol +++ b/src/migration/AaveV2MigrationBundler.sol @@ -28,6 +28,7 @@ contract AaveV2MigrationBundler is MigrationBundler { /// @notice Repays `amount` of `asset` on AaveV2, on behalf of the initiator. /// @notice Warning: should only be called via the bundler's `multicall` function. + /// @dev Warning: `asset` can re-enter the bundler flow. /// @dev Pass `amount = type(uint256).max` to repay all. function aaveV2Repay(address asset, uint256 amount, uint256 interestRateMode) external payable { amount = Math.min(amount, ERC20(asset).balanceOf(address(this))); diff --git a/src/migration/AaveV3MigrationBundler.sol b/src/migration/AaveV3MigrationBundler.sol index a15fe12f..dcb1206f 100644 --- a/src/migration/AaveV3MigrationBundler.sol +++ b/src/migration/AaveV3MigrationBundler.sol @@ -27,6 +27,7 @@ contract AaveV3MigrationBundler is MigrationBundler { /// @notice Repays `amount` of `asset` on AaveV3, on behalf of the initiator. /// @notice Warning: should only be called via the bundler's `multicall` function. + /// @dev Warning: `asset` can re-enter the bundler flow. /// @dev Pass `amount = type(uint256).max` to repay all. function aaveV3Repay(address asset, uint256 amount, uint256 interestRateMode) external payable { amount = Math.min(amount, ERC20(asset).balanceOf(address(this))); diff --git a/src/migration/AaveV3OptimizerMigrationBundler.sol b/src/migration/AaveV3OptimizerMigrationBundler.sol index 48b56ec0..dfeabfa4 100644 --- a/src/migration/AaveV3OptimizerMigrationBundler.sol +++ b/src/migration/AaveV3OptimizerMigrationBundler.sol @@ -27,6 +27,7 @@ contract AaveV3OptimizerMigrationBundler is MigrationBundler { /// @notice Repays `amount` of `underlying` on the AaveV3 Optimizer, on behalf of the initiator. /// @notice Warning: should only be called via the bundler's `multicall` function. + /// @dev Warning: `underlying` can re-enter the bundler flow. /// @dev Pass `amount = type(uint256).max` to repay all. function aaveV3OptimizerRepay(address underlying, uint256 amount) external payable { amount = Math.min(amount, ERC20(underlying).balanceOf(address(this))); diff --git a/src/migration/CompoundV2MigrationBundler.sol b/src/migration/CompoundV2MigrationBundler.sol index b5a46cbf..67de2bbf 100644 --- a/src/migration/CompoundV2MigrationBundler.sol +++ b/src/migration/CompoundV2MigrationBundler.sol @@ -37,6 +37,7 @@ contract CompoundV2MigrationBundler is WNativeBundler, MigrationBundler { /// @notice Repays `amount` of `cToken`'s underlying asset, on behalf of the initiator. /// @notice Warning: should only be called via the bundler's `multicall` function. + /// @dev Warning: `cToken` can re-enter the bundler flow. /// @dev Pass `amount = type(uint256).max` to repay all. function compoundV2Repay(address cToken, uint256 amount) external payable { if (cToken == C_ETH) { @@ -62,6 +63,7 @@ contract CompoundV2MigrationBundler is WNativeBundler, MigrationBundler { /// @notice Redeems `amount` of `cToken` from CompoundV2. /// @dev Initiator must have previously transferred their cTokens to the bundler. + /// @dev Warning: `cToken` can re-enter the bundler flow. /// @dev Pass `amount = type(uint256).max` to redeem all. function compoundV2Redeem(address cToken, uint256 amount) external payable { amount = Math.min(amount, ERC20(cToken).balanceOf(address(this))); diff --git a/src/migration/CompoundV3MigrationBundler.sol b/src/migration/CompoundV3MigrationBundler.sol index 9bd7fd97..65756ed6 100644 --- a/src/migration/CompoundV3MigrationBundler.sol +++ b/src/migration/CompoundV3MigrationBundler.sol @@ -21,6 +21,7 @@ contract CompoundV3MigrationBundler is MigrationBundler { /// @notice Repays `amount` of `asset` on the CompoundV3 `instance`, on behalf of the initiator. /// @notice Warning: should only be called via the bundler's `multicall` function. + /// @dev Warning: `instance` can re-enter the bundler flow. /// @dev Assumes the given instance is a CompoundV3 instance. /// @dev Pass `amount = type(uint256).max` to repay all. function compoundV3Repay(address instance, address asset, uint256 amount) external payable { @@ -36,6 +37,7 @@ contract CompoundV3MigrationBundler is MigrationBundler { /// @notice Withdraws `amount` of `asset` on the CompoundV3 `instance`. /// @dev Initiator must have previously transferred their CompoundV3 position to the bundler. + /// @dev Warning: `instance` can re-enter the bundler flow. /// @dev Assumes the given `instance` is a CompoundV3 instance. /// @dev Pass `amount = type(uint256).max` to withdraw all. function compoundV3Withdraw(address instance, address asset, uint256 amount) external payable { @@ -45,6 +47,7 @@ contract CompoundV3MigrationBundler is MigrationBundler { /// @notice Withdraws `amount` of `asset` from the CompoundV3 `instance`, on behalf of the initiator. /// @notice Warning: should only be called via the bundler's `multicall` function. /// @dev Initiator must have previously approved the bundler to manage their CompoundV3 position. + /// @dev Warning: `instance` can re-enter the bundler flow. /// @dev Assumes the given `instance` is a CompoundV3 instance. /// @dev Pass `amount = type(uint256).max` to withdraw all. function compoundV3WithdrawFrom(address instance, address asset, uint256 amount) external payable { @@ -54,6 +57,7 @@ contract CompoundV3MigrationBundler is MigrationBundler { /// @notice Approves the bundler to act on behalf of the initiator on the CompoundV3 `instance`, given a signed /// EIP-712 approval message. /// @notice Warning: should only be called via the bundler's `multicall` function. + /// @dev Warning: `instance` can re-enter the bundler flow. /// @dev Assumes the given `instance` is a CompoundV3 instance. function compoundV3AllowBySig( address instance, From cfd86bbf0e516438ba0da3b0ef74ca9a05bb4b5f Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Tue, 31 Oct 2023 10:20:25 +0300 Subject: [PATCH 2/3] docs: remove useless comments --- src/ERC4626Bundler.sol | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/ERC4626Bundler.sol b/src/ERC4626Bundler.sol index 3e4662d4..c4c99dd6 100644 --- a/src/ERC4626Bundler.sol +++ b/src/ERC4626Bundler.sol @@ -21,7 +21,6 @@ abstract contract ERC4626Bundler is BaseBundler { /// @notice Mints the given amount of `shares` on the given ERC4626 `vault`, on behalf of `receiver`. /// @dev Warning: `vault` can re-enter the bundler flow. /// @dev Assumes the given `vault` implements EIP-4626. - /// @dev Pass `type(uint256).max` as `shares` to mint max. /// @param vault The address of the vault. /// @param shares The amount of shares to mint. Pass `type(uint256).max` to mint max. /// @param receiver The address to which shares will be minted. @@ -45,7 +44,6 @@ abstract contract ERC4626Bundler is BaseBundler { /// @notice Deposits the given amount of `assets` on the given ERC4626 `vault`, on behalf of `receiver`. /// @dev Warning: `vault` can re-enter the bundler flow. /// @dev Assumes the given `vault` implements EIP-4626. - /// @dev Pass `type(uint256).max` as `assets` to deposit max. /// @param vault The address of the vault. /// @param assets The amount of assets to deposit. Pass `type(uint256).max` to deposit max. /// @param receiver The address to which shares will be minted. @@ -71,7 +69,6 @@ abstract contract ERC4626Bundler is BaseBundler { /// @notice Warning: should only be called via the bundler's `multicall` function. /// @dev Warning: `vault` can re-enter the bundler flow. /// @dev Assumes the given `vault` implements EIP-4626. - /// @dev Pass `type(uint256).max` as `assets` to withdraw max. /// @param vault The address of the vault. /// @param assets The amount of assets to withdraw. Pass `type(uint256).max` to withdraw max. /// @param receiver The address that will receive the withdrawn assets. @@ -92,7 +89,6 @@ abstract contract ERC4626Bundler is BaseBundler { /// @notice Warning: should only be called via the bundler's `multicall` function. /// @dev Warning: `vault` can re-enter the bundler flow. /// @dev Assumes the given `vault` implements EIP-4626. - /// @dev Pass `type(uint256).max` as `shares` to redeem max. /// @param vault The address of the vault. /// @param shares The amount of shares to burn. Pass `type(uint256).max` to redeem max. /// @param receiver The address that will receive the withdrawn assets. From d87da3949c1ad6a8779a484277f937db81f35504 Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Tue, 31 Oct 2023 11:30:19 +0300 Subject: [PATCH 3/3] docs: remove useless comment again --- src/PermitBundler.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/src/PermitBundler.sol b/src/PermitBundler.sol index 27986c62..53555cb8 100644 --- a/src/PermitBundler.sol +++ b/src/PermitBundler.sol @@ -14,7 +14,6 @@ abstract contract PermitBundler is BaseBundler { /// the given `deadline` & EIP-712 signature's `v`, `r` & `s`. /// @notice Warning: should only be called via the bundler's `multicall` function. /// @dev Warning: `asset` can re-enter the bundler flow. - /// @dev Pass `skipRevert = true` to avoid reverting the whole bundle in case the signature expired. /// @param asset The address of the token to be permitted. /// @param amount The amount of `asset` to be permitted. /// @param deadline The deadline of the approval.