From 0a581d282985706403e8a9a6e3ccf90f9a419577 Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Mon, 18 Sep 2023 06:43:23 +0200 Subject: [PATCH 1/2] refactor(errors): add vault & instance checks --- contracts/ERC4626Bundler.sol | 4 ++++ contracts/URDBundler.sol | 2 +- contracts/migration/CompoundV3MigrationBundler.sol | 10 ++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/contracts/ERC4626Bundler.sol b/contracts/ERC4626Bundler.sol index 7c367699..c6c92bc7 100644 --- a/contracts/ERC4626Bundler.sol +++ b/contracts/ERC4626Bundler.sol @@ -20,6 +20,7 @@ abstract contract ERC4626Bundler is BaseBundler { /// @notice Mints the given amount of `shares` on the given ERC4626 `vault`, on behalf of `owner`. function erc4626Mint(address vault, uint256 shares, address owner) external payable { + require(vault != address(0), ErrorsLib.ZERO_ADDRESS); require(owner != address(0), ErrorsLib.ZERO_ADDRESS); address asset = IERC4626(vault).asset(); @@ -38,6 +39,7 @@ abstract contract ERC4626Bundler is BaseBundler { /// @notice Deposits the given amount of `assets` on the given ERC4626 `vault`, on behalf of `owner`. function erc4626Deposit(address vault, uint256 assets, address owner) external payable { + require(vault != address(0), ErrorsLib.ZERO_ADDRESS); require(owner != address(0), ErrorsLib.ZERO_ADDRESS); address asset = IERC4626(vault).asset(); @@ -57,6 +59,7 @@ abstract contract ERC4626Bundler is BaseBundler { /// `receiver`. /// @notice Warning: should only be called via the bundler's `multicall` function. function erc4626Withdraw(address vault, uint256 assets, address receiver) external payable { + require(vault != address(0), ErrorsLib.ZERO_ADDRESS); require(receiver != address(0), ErrorsLib.ZERO_ADDRESS); address initiator = _initiator; @@ -70,6 +73,7 @@ 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. function erc4626Redeem(address vault, uint256 shares, address receiver) external payable { + require(vault != address(0), ErrorsLib.ZERO_ADDRESS); require(receiver != address(0), ErrorsLib.ZERO_ADDRESS); address initiator = _initiator; diff --git a/contracts/URDBundler.sol b/contracts/URDBundler.sol index 0211e59a..e6207fd9 100644 --- a/contracts/URDBundler.sol +++ b/contracts/URDBundler.sol @@ -24,8 +24,8 @@ contract URDBundler is BaseBundler { external payable { - require(account != address(this), ErrorsLib.BUNDLER_ADDRESS); require(account != address(0), ErrorsLib.ZERO_ADDRESS); + require(account != address(this), ErrorsLib.BUNDLER_ADDRESS); URD.claim(distributionId, account, reward, claimable, proof); } diff --git a/contracts/migration/CompoundV3MigrationBundler.sol b/contracts/migration/CompoundV3MigrationBundler.sol index 05de3bcc..785b2f51 100644 --- a/contracts/migration/CompoundV3MigrationBundler.sol +++ b/contracts/migration/CompoundV3MigrationBundler.sol @@ -3,6 +3,8 @@ pragma solidity 0.8.21; import {ICompoundV3} from "./interfaces/ICompoundV3.sol"; +import {ErrorsLib} from "../libraries/ErrorsLib.sol"; + import {Permit2Bundler} from "../Permit2Bundler.sol"; import {MigrationBundler} from "./MigrationBundler.sol"; @@ -20,6 +22,8 @@ contract CompoundV3MigrationBundler is MigrationBundler, Permit2Bundler { /// @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. function compoundV3Repay(address instance, address asset, uint256 amount) external payable { + require(instance != address(0), ErrorsLib.ZERO_ADDRESS); + _approveMaxTo(asset, instance); // Compound V3 uses signed accounting: supplying to a negative balance actually repays the borrow position. @@ -29,6 +33,8 @@ contract CompoundV3MigrationBundler is MigrationBundler, Permit2Bundler { /// @notice Withdraws `amount` of `asset` on the CompoundV3 `instance`. /// @dev Initiator must have previously transferred their CompoundV3 position to the bundler. function compoundV3Withdraw(address instance, address asset, uint256 amount) external payable { + require(instance != address(0), ErrorsLib.ZERO_ADDRESS); + ICompoundV3(instance).withdraw(asset, amount); } @@ -36,6 +42,8 @@ contract CompoundV3MigrationBundler is MigrationBundler, Permit2Bundler { /// @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. function compoundV3WithdrawFrom(address instance, address asset, uint256 amount) external payable { + require(instance != address(0), ErrorsLib.ZERO_ADDRESS); + ICompoundV3(instance).withdrawFrom(_initiator, address(this), asset, amount); } @@ -51,6 +59,8 @@ contract CompoundV3MigrationBundler is MigrationBundler, Permit2Bundler { bytes32 r, bytes32 s ) external payable { + require(instance != address(0), ErrorsLib.ZERO_ADDRESS); + ICompoundV3(instance).allowBySig(_initiator, address(this), isAllowed, nonce, expiry, v, r, s); } } From b35535e252759175e455e8fccbc0906a591f229b Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Mon, 18 Sep 2023 06:56:02 +0200 Subject: [PATCH 2/2] test(erc4626): add zero target test --- test/forge/ERC4626BundlerLocalTest.sol | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/forge/ERC4626BundlerLocalTest.sol b/test/forge/ERC4626BundlerLocalTest.sol index 112d79da..2942e4c6 100644 --- a/test/forge/ERC4626BundlerLocalTest.sol +++ b/test/forge/ERC4626BundlerLocalTest.sol @@ -21,6 +21,13 @@ contract ERC4626BundlerLocalTest is LocalTest { bundler = new ERC4626BundlerMock(); } + function testErc4626MintZeroAdressTarget(uint256 shares) public { + bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Mint, (address(0), shares, RECEIVER))); + + vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS)); + bundler.multicall(block.timestamp, bundle); + } + function testErc4626MintZeroAdress(uint256 shares) public { bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Mint, (address(vault), shares, address(0)))); @@ -28,6 +35,13 @@ contract ERC4626BundlerLocalTest is LocalTest { bundler.multicall(block.timestamp, bundle); } + function testErc4626DepositZeroAdressTarget(uint256 assets) public { + bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Deposit, (address(0), assets, RECEIVER))); + + vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS)); + bundler.multicall(block.timestamp, bundle); + } + function testErc4626DepositZeroAdress(uint256 assets) public { bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Deposit, (address(vault), assets, address(0)))); @@ -35,6 +49,13 @@ contract ERC4626BundlerLocalTest is LocalTest { bundler.multicall(block.timestamp, bundle); } + function testErc4626WithdrawZeroAdressTarget(uint256 assets) public { + bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Withdraw, (address(0), assets, RECEIVER))); + + vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS)); + bundler.multicall(block.timestamp, bundle); + } + function testErc4626WithdrawZeroAdress(uint256 assets) public { bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Withdraw, (address(vault), assets, address(0)))); @@ -42,6 +63,13 @@ contract ERC4626BundlerLocalTest is LocalTest { bundler.multicall(block.timestamp, bundle); } + function testErc4626RedeemZeroAdressTarget(uint256 assets) public { + bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Redeem, (address(0), assets, RECEIVER))); + + vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS)); + bundler.multicall(block.timestamp, bundle); + } + function testErc4626RedeemZeroAdress(uint256 shares) public { bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Redeem, (address(vault), shares, address(0))));