From 68d50d1f189f69e7705fde3bbf3eb76922809f92 Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Thu, 14 May 2026 19:56:19 +1000 Subject: [PATCH] Use safe transfers --- src/contracts/AbstractARM.sol | 24 +- .../adapters/AbstractLidoAssetAdapter.sol | 10 +- src/contracts/adapters/EthenaAssetAdapter.sol | 9 +- .../adapters/EtherFiAssetAdapter.sol | 12 +- src/contracts/adapters/OriginAssetAdapter.sol | 12 +- src/contracts/adapters/StETHAssetAdapter.sol | 8 +- src/contracts/adapters/WeETHAssetAdapter.sol | 12 +- .../adapters/WrappedOriginAssetAdapter.sol | 12 +- src/contracts/adapters/WstETHAssetAdapter.sol | 9 +- test/unit/OriginARM/ERC20Safety.sol | 247 ++++++++++++++++++ test/unit/mocks/MockFalseReturnERC20.sol | 39 +++ 11 files changed, 359 insertions(+), 35 deletions(-) create mode 100644 test/unit/OriginARM/ERC20Safety.sol create mode 100644 test/unit/mocks/MockFalseReturnERC20.sol diff --git a/src/contracts/AbstractARM.sol b/src/contracts/AbstractARM.sol index 2f7ddf81..2c54199b 100644 --- a/src/contracts/AbstractARM.sol +++ b/src/contracts/AbstractARM.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.23; import {ERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; import {IERC4626} from "@openzeppelin/contracts/interfaces/IERC4626.sol"; +import {IERC20 as OZIERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import {OwnableOperable} from "./OwnableOperable.sol"; @@ -17,6 +19,8 @@ import {IAssetAdapter, IERC20, ICapManager} from "./Interfaces.sol"; * @author Origin Protocol Inc */ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable { + using SafeERC20 for OZIERC20; + //////////////////////////////////////////////////// /// Constants //////////////////////////////////////////////////// @@ -213,7 +217,7 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable { __ERC20_init(_name, _symbol); // Transfer a small bit of liquidity from the initializer to this contract. - IERC20(liquidityAsset).transferFrom(msg.sender, address(this), MIN_TOTAL_SUPPLY); + OZIERC20(liquidityAsset).safeTransferFrom(msg.sender, address(this), MIN_TOTAL_SUPPLY); // Mint a small amount of shares to a dead account so total supply can never be zero. // This avoids donation attacks when there are no assets in the ARM contract. _mint(DEAD_ACCOUNT, MIN_TOTAL_SUPPLY); @@ -369,10 +373,10 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable { _consumeSwapLiquidityLimit(config, isBuySide, amountOut); // Transfer the input tokens from the caller to this ARM contract - inToken.transferFrom(msg.sender, address(this), amountIn); + OZIERC20(address(inToken)).safeTransferFrom(msg.sender, address(this), amountIn); // Transfer the output tokens to the recipient - outToken.transfer(to, amountOut); + OZIERC20(address(outToken)).safeTransfer(to, amountOut); } /// @dev Swap for exact output between the liquidity asset and one supported base asset. @@ -412,10 +416,10 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable { _consumeSwapLiquidityLimit(config, isBuySide, amountOut); // Transfer the input tokens from the caller to this ARM contract - inToken.transferFrom(msg.sender, address(this), amountIn); + OZIERC20(address(inToken)).safeTransferFrom(msg.sender, address(this), amountIn); // Transfer the output tokens to the recipient - outToken.transfer(to, amountOut); + OZIERC20(address(outToken)).safeTransfer(to, amountOut); } /// @dev Resolve the supported base asset from a 2-token swap pair. @@ -536,7 +540,7 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable { baseAssets.push(newBaseAsset); // Allow the adapter to pull base assets when requesting protocol redemptions. - IERC20(newBaseAsset).approve(adapter, type(uint256).max); + OZIERC20(newBaseAsset).forceApprove(adapter, type(uint256).max); baseAssetConfigs[newBaseAsset] = BaseAssetConfig({ buyPrice: SafeCast.toUint128(buyPrice), sellPrice: SafeCast.toUint128(sellPrice), @@ -681,7 +685,7 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable { shares = convertToShares(assets); // Transfer liquidity from the depositor before minting LP shares. - IERC20(liquidityAsset).transferFrom(msg.sender, address(this), assets); + OZIERC20(liquidityAsset).safeTransferFrom(msg.sender, address(this), assets); _mint(receiver, shares); // Enforce LP caps after the deposit has changed the receiver's share balance. @@ -774,7 +778,7 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable { } // Transfer the liquidity asset to the withdrawer. - IERC20(liquidityAsset).transfer(msg.sender, assets); + OZIERC20(liquidityAsset).safeTransfer(msg.sender, assets); emit RedeemClaimed(msg.sender, requestId, assets); } @@ -960,7 +964,7 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable { require(fees <= IERC20(liquidityAsset).balanceOf(address(this)), "ARM: insufficient liquidity"); feesAccrued = 0; - IERC20(liquidityAsset).transfer(feeCollector, fees); + OZIERC20(liquidityAsset).safeTransfer(feeCollector, fees); emit FeeCollected(feeCollector, fees); } @@ -1060,7 +1064,7 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable { if (targetLiquidityDelta > allocateThreshold) { // We have too much liquidity in the ARM, so deposit some to the active lending market. uint256 depositAmount = SafeCast.toUint256(targetLiquidityDelta); - IERC20(liquidityAsset).approve(activeMarketMem, depositAmount); + OZIERC20(liquidityAsset).forceApprove(activeMarketMem, depositAmount); IERC4626(activeMarketMem).deposit(depositAmount, address(this)); actualLiquidityDelta = SafeCast.toInt256(depositAmount); } else if (targetLiquidityDelta < 0) { diff --git a/src/contracts/adapters/AbstractLidoAssetAdapter.sol b/src/contracts/adapters/AbstractLidoAssetAdapter.sol index 81eb9b01..eac9b065 100644 --- a/src/contracts/adapters/AbstractLidoAssetAdapter.sol +++ b/src/contracts/adapters/AbstractLidoAssetAdapter.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.23; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import {IERC20 as OZIERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {IAssetAdapter, IERC20, IStETHWithdrawal, ISTETH, IWETH} from "../Interfaces.sol"; @@ -10,6 +12,8 @@ interface ILidoARMLegacyQueueCheck { } abstract contract AbstractLidoAssetAdapter is Initializable, IAssetAdapter { + using SafeERC20 for OZIERC20; + uint256 internal constant MAX_WITHDRAWAL_AMOUNT = 1000 ether; address public immutable arm; @@ -29,12 +33,12 @@ abstract contract AbstractLidoAssetAdapter is Initializable, IAssetAdapter { steth = ISTETH(_steth); lidoWithdrawalQueue = IStETHWithdrawal(_lidoWithdrawalQueue); - IERC20(_steth).approve(_lidoWithdrawalQueue, type(uint256).max); + OZIERC20(_steth).forceApprove(_lidoWithdrawalQueue, type(uint256).max); } function initialize() external initializer { ILidoARMLegacyQueueCheck(arm).checkNoLegacyLidoWithdrawalRequests(); - IERC20(address(steth)).approve(address(lidoWithdrawalQueue), type(uint256).max); + OZIERC20(address(steth)).forceApprove(address(lidoWithdrawalQueue), type(uint256).max); } function asset() external view returns (address) { @@ -115,7 +119,7 @@ abstract contract AbstractLidoAssetAdapter is Initializable, IAssetAdapter { } assetsReceived = weth.balanceOf(address(this)) - wethBefore; - IERC20(address(weth)).transfer(arm, assetsReceived); + OZIERC20(address(weth)).safeTransfer(arm, assetsReceived); } function claimableRedeem() external view returns (uint256 claimableShares, uint256 claimableAssets) { diff --git a/src/contracts/adapters/EthenaAssetAdapter.sol b/src/contracts/adapters/EthenaAssetAdapter.sol index 64fa8f99..e3631f46 100644 --- a/src/contracts/adapters/EthenaAssetAdapter.sol +++ b/src/contracts/adapters/EthenaAssetAdapter.sol @@ -1,11 +1,16 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.23; +import {IERC20 as OZIERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; + import {EthenaUnstaker} from "../EthenaUnstaker.sol"; import {IAssetAdapter, IERC20, IStakedUSDe, UserCooldown} from "../Interfaces.sol"; import {Ownable} from "../Ownable.sol"; contract EthenaAssetAdapter is IAssetAdapter, Ownable { + using SafeERC20 for OZIERC20; + uint256 public constant DELAY_REQUEST = 30 minutes; uint8 public constant MAX_UNSTAKERS = 42; @@ -61,7 +66,7 @@ contract EthenaAssetAdapter is IAssetAdapter, Ownable { pendingUnstakerIndexes.push(nextUnstakerIndex); nextUnstakerIndex = uint8((nextUnstakerIndex + 1) % MAX_UNSTAKERS); - susde.transferFrom(arm, unstaker, shares); + OZIERC20(address(susde)).safeTransferFrom(arm, unstaker, shares); assetsExpected = EthenaUnstaker(unstaker).requestUnstake(shares); requestShares[unstaker] = shares; requestAssets[unstaker] = assetsExpected; @@ -101,7 +106,7 @@ contract EthenaAssetAdapter is IAssetAdapter, Ownable { nextPendingIndex = cursor + claimCount; assetsReceived = usde.balanceOf(address(this)) - balanceBefore; - usde.transfer(arm, assetsReceived); + OZIERC20(address(usde)).safeTransfer(arm, assetsReceived); } function deployUnstakers() external onlyOwner { diff --git a/src/contracts/adapters/EtherFiAssetAdapter.sol b/src/contracts/adapters/EtherFiAssetAdapter.sol index bff5423d..046b4b5f 100644 --- a/src/contracts/adapters/EtherFiAssetAdapter.sol +++ b/src/contracts/adapters/EtherFiAssetAdapter.sol @@ -3,10 +3,14 @@ pragma solidity ^0.8.23; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; +import {IERC20 as OZIERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {IAssetAdapter, IERC20, IEETHWithdrawal, IEETHWithdrawalNFT, IWETH} from "../Interfaces.sol"; contract EtherFiAssetAdapter is Initializable, IAssetAdapter, IERC721Receiver { + using SafeERC20 for OZIERC20; + address public immutable arm; IERC20 public immutable eeth; IWETH public immutable weth; @@ -30,11 +34,11 @@ contract EtherFiAssetAdapter is Initializable, IAssetAdapter, IERC721Receiver { etherfiWithdrawalQueue = IEETHWithdrawal(_etherfiWithdrawalQueue); etherfiWithdrawalNFT = IEETHWithdrawalNFT(_etherfiWithdrawalNFT); - eeth.approve(_etherfiWithdrawalQueue, type(uint256).max); + OZIERC20(address(eeth)).forceApprove(_etherfiWithdrawalQueue, type(uint256).max); } function initialize() external initializer { - eeth.approve(address(etherfiWithdrawalQueue), type(uint256).max); + OZIERC20(address(eeth)).forceApprove(address(etherfiWithdrawalQueue), type(uint256).max); } function asset() external view returns (address) { @@ -55,7 +59,7 @@ contract EtherFiAssetAdapter is Initializable, IAssetAdapter, IERC721Receiver { nonZeroShares(shares) returns (uint256 sharesRequested, uint256 assetsExpected) { - eeth.transferFrom(arm, address(this), shares); + OZIERC20(address(eeth)).safeTransferFrom(arm, address(this), shares); uint256 requestId = etherfiWithdrawalQueue.requestWithdraw(address(this), shares); requestShares[requestId] = shares; pendingRequestIds.push(requestId); @@ -101,7 +105,7 @@ contract EtherFiAssetAdapter is Initializable, IAssetAdapter, IERC721Receiver { if (ethBalance > 0) weth.deposit{value: ethBalance}(); assetsReceived = weth.balanceOf(address(this)) - wethBefore; - IERC20(address(weth)).transfer(arm, assetsReceived); + OZIERC20(address(weth)).safeTransfer(arm, assetsReceived); } function pendingRequestIdsLength() external view returns (uint256) { diff --git a/src/contracts/adapters/OriginAssetAdapter.sol b/src/contracts/adapters/OriginAssetAdapter.sol index 6d155f61..b8329928 100644 --- a/src/contracts/adapters/OriginAssetAdapter.sol +++ b/src/contracts/adapters/OriginAssetAdapter.sol @@ -2,10 +2,14 @@ pragma solidity ^0.8.23; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import {IERC20 as OZIERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {IAssetAdapter, IERC20, IOriginVault} from "../Interfaces.sol"; contract OriginAssetAdapter is Initializable, IAssetAdapter { + using SafeERC20 for OZIERC20; + address public immutable arm; IERC20 public immutable otoken; IERC20 public immutable liquidityAsset; @@ -20,11 +24,11 @@ contract OriginAssetAdapter is Initializable, IAssetAdapter { otoken = IERC20(_otoken); liquidityAsset = IERC20(_liquidityAsset); vault = IOriginVault(_vault); - otoken.approve(_vault, type(uint256).max); + OZIERC20(address(otoken)).forceApprove(_vault, type(uint256).max); } function initialize() external initializer { - otoken.approve(address(vault), type(uint256).max); + OZIERC20(address(otoken)).forceApprove(address(vault), type(uint256).max); } function asset() external view returns (address) { @@ -45,7 +49,7 @@ contract OriginAssetAdapter is Initializable, IAssetAdapter { nonZeroShares(shares) returns (uint256 sharesRequested, uint256 assetsExpected) { - otoken.transferFrom(arm, address(this), shares); + OZIERC20(address(otoken)).safeTransferFrom(arm, address(this), shares); (uint256 requestId,) = vault.requestWithdrawal(shares); requestShares[requestId] = shares; pendingRequestIds.push(requestId); @@ -88,7 +92,7 @@ contract OriginAssetAdapter is Initializable, IAssetAdapter { (, uint256 amountClaimed) = vault.claimWithdrawals(requestIds); uint256 balanceDelta = liquidityAsset.balanceOf(address(this)) - balanceBefore; assetsReceived = balanceDelta > amountClaimed ? balanceDelta : amountClaimed; - liquidityAsset.transfer(arm, balanceDelta); + OZIERC20(address(liquidityAsset)).safeTransfer(arm, balanceDelta); } function pendingRequestIdsLength() external view returns (uint256) { diff --git a/src/contracts/adapters/StETHAssetAdapter.sol b/src/contracts/adapters/StETHAssetAdapter.sol index 2f4e1037..aea56cac 100644 --- a/src/contracts/adapters/StETHAssetAdapter.sol +++ b/src/contracts/adapters/StETHAssetAdapter.sol @@ -1,10 +1,14 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.23; -import {IERC20} from "../Interfaces.sol"; +import {IERC20 as OZIERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; + import {AbstractLidoAssetAdapter} from "./AbstractLidoAssetAdapter.sol"; contract StETHAssetAdapter is AbstractLidoAssetAdapter { + using SafeERC20 for OZIERC20; + constructor(address _arm, address _weth, address _steth, address _lidoWithdrawalQueue) AbstractLidoAssetAdapter(_arm, _weth, _steth, _lidoWithdrawalQueue) {} @@ -18,7 +22,7 @@ contract StETHAssetAdapter is AbstractLidoAssetAdapter { } function _pullSharesAndConvertToSteth(address owner, uint256 shares) internal override returns (uint256 assetsOut) { - IERC20(address(steth)).transferFrom(owner, address(this), shares); + OZIERC20(address(steth)).safeTransferFrom(owner, address(this), shares); assetsOut = shares; } diff --git a/src/contracts/adapters/WeETHAssetAdapter.sol b/src/contracts/adapters/WeETHAssetAdapter.sol index 9d59de72..ba10ef3d 100644 --- a/src/contracts/adapters/WeETHAssetAdapter.sol +++ b/src/contracts/adapters/WeETHAssetAdapter.sol @@ -3,10 +3,14 @@ pragma solidity ^0.8.23; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; +import {IERC20 as OZIERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {IAssetAdapter, IERC20, IEETHWithdrawal, IEETHWithdrawalNFT, IWeETH, IWETH} from "../Interfaces.sol"; contract WeETHAssetAdapter is Initializable, IAssetAdapter, IERC721Receiver { + using SafeERC20 for OZIERC20; + address public immutable arm; IWeETH public immutable weeth; IERC20 public immutable eeth; @@ -34,11 +38,11 @@ contract WeETHAssetAdapter is Initializable, IAssetAdapter, IERC721Receiver { etherfiWithdrawalQueue = IEETHWithdrawal(_etherfiWithdrawalQueue); etherfiWithdrawalNFT = IEETHWithdrawalNFT(_etherfiWithdrawalNFT); - eeth.approve(_etherfiWithdrawalQueue, type(uint256).max); + OZIERC20(address(eeth)).forceApprove(_etherfiWithdrawalQueue, type(uint256).max); } function initialize() external initializer { - eeth.approve(address(etherfiWithdrawalQueue), type(uint256).max); + OZIERC20(address(eeth)).forceApprove(address(etherfiWithdrawalQueue), type(uint256).max); } function asset() external view returns (address) { @@ -59,7 +63,7 @@ contract WeETHAssetAdapter is Initializable, IAssetAdapter, IERC721Receiver { nonZeroShares(shares) returns (uint256 sharesRequested, uint256 assetsExpected) { - IERC20(address(weeth)).transferFrom(arm, address(this), shares); + OZIERC20(address(weeth)).safeTransferFrom(arm, address(this), shares); assetsExpected = weeth.unwrap(shares); uint256 requestId = etherfiWithdrawalQueue.requestWithdraw(address(this), assetsExpected); @@ -108,7 +112,7 @@ contract WeETHAssetAdapter is Initializable, IAssetAdapter, IERC721Receiver { if (ethBalance > 0) weth.deposit{value: ethBalance}(); assetsReceived = weth.balanceOf(address(this)) - wethBefore; - IERC20(address(weth)).transfer(arm, assetsReceived); + OZIERC20(address(weth)).safeTransfer(arm, assetsReceived); } function pendingRequestIdsLength() external view returns (uint256) { diff --git a/src/contracts/adapters/WrappedOriginAssetAdapter.sol b/src/contracts/adapters/WrappedOriginAssetAdapter.sol index 11e4dae3..91570ea3 100644 --- a/src/contracts/adapters/WrappedOriginAssetAdapter.sol +++ b/src/contracts/adapters/WrappedOriginAssetAdapter.sol @@ -3,10 +3,14 @@ pragma solidity ^0.8.23; import {IERC4626} from "@openzeppelin/contracts/interfaces/IERC4626.sol"; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import {IERC20 as OZIERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {IAssetAdapter, IERC20, IOriginVault} from "../Interfaces.sol"; contract WrappedOriginAssetAdapter is Initializable, IAssetAdapter { + using SafeERC20 for OZIERC20; + address public immutable arm; IERC4626 public immutable wrappedOToken; IERC20 public immutable otoken; @@ -24,11 +28,11 @@ contract WrappedOriginAssetAdapter is Initializable, IAssetAdapter { otoken = IERC20(_otoken); liquidityAsset = IERC20(_liquidityAsset); vault = IOriginVault(_vault); - otoken.approve(_vault, type(uint256).max); + OZIERC20(address(otoken)).forceApprove(_vault, type(uint256).max); } function initialize() external initializer { - otoken.approve(address(vault), type(uint256).max); + OZIERC20(address(otoken)).forceApprove(address(vault), type(uint256).max); } function asset() external view returns (address) { @@ -49,7 +53,7 @@ contract WrappedOriginAssetAdapter is Initializable, IAssetAdapter { nonZeroShares(shares) returns (uint256 sharesRequested, uint256 assetsExpected) { - IERC20(address(wrappedOToken)).transferFrom(arm, address(this), shares); + OZIERC20(address(wrappedOToken)).safeTransferFrom(arm, address(this), shares); assetsExpected = wrappedOToken.redeem(shares, address(this), address(this)); (uint256 requestId,) = vault.requestWithdrawal(assetsExpected); @@ -95,7 +99,7 @@ contract WrappedOriginAssetAdapter is Initializable, IAssetAdapter { (, uint256 amountClaimed) = vault.claimWithdrawals(requestIds); uint256 balanceDelta = liquidityAsset.balanceOf(address(this)) - balanceBefore; assetsReceived = balanceDelta > amountClaimed ? balanceDelta : amountClaimed; - liquidityAsset.transfer(arm, balanceDelta); + OZIERC20(address(liquidityAsset)).safeTransfer(arm, balanceDelta); } function pendingRequestIdsLength() external view returns (uint256) { diff --git a/src/contracts/adapters/WstETHAssetAdapter.sol b/src/contracts/adapters/WstETHAssetAdapter.sol index 36e77333..5b03e817 100644 --- a/src/contracts/adapters/WstETHAssetAdapter.sol +++ b/src/contracts/adapters/WstETHAssetAdapter.sol @@ -1,10 +1,15 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.23; -import {IERC20, IWstETH} from "../Interfaces.sol"; +import {IERC20 as OZIERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; + +import {IWstETH} from "../Interfaces.sol"; import {AbstractLidoAssetAdapter} from "./AbstractLidoAssetAdapter.sol"; contract WstETHAssetAdapter is AbstractLidoAssetAdapter { + using SafeERC20 for OZIERC20; + IWstETH public immutable wsteth; constructor(address _arm, address _weth, address _steth, address _wsteth, address _lidoWithdrawalQueue) @@ -22,7 +27,7 @@ contract WstETHAssetAdapter is AbstractLidoAssetAdapter { } function _pullSharesAndConvertToSteth(address owner, uint256 shares) internal override returns (uint256 assetsOut) { - IERC20(address(wsteth)).transferFrom(owner, address(this), shares); + OZIERC20(address(wsteth)).safeTransferFrom(owner, address(this), shares); assetsOut = wsteth.unwrap(shares); } diff --git a/test/unit/OriginARM/ERC20Safety.sol b/test/unit/OriginARM/ERC20Safety.sol new file mode 100644 index 00000000..9ec00261 --- /dev/null +++ b/test/unit/OriginARM/ERC20Safety.sol @@ -0,0 +1,247 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.23; + +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import {MockERC20} from "@solmate/test/utils/mocks/MockERC20.sol"; + +import {OriginARM} from "contracts/OriginARM.sol"; +import {OriginAssetAdapter} from "contracts/adapters/OriginAssetAdapter.sol"; +import {IERC20} from "contracts/Interfaces.sol"; +import {Unit_Shared_Test} from "test/unit/shared/Shared.sol"; +import {MockFalseReturnERC20} from "test/unit/mocks/MockFalseReturnERC20.sol"; + +contract Unit_Concrete_OriginARM_ERC20Safety_Test_ is Unit_Shared_Test { + function test_RevertWhen_Deposit_TransferFromReturnsFalse() public { + deal(address(weth), alice, DEFAULT_AMOUNT); + + vm.prank(alice); + weth.approve(address(originARM), DEFAULT_AMOUNT); + + vm.mockCall( + address(weth), + abi.encodeWithSelector(IERC20.transferFrom.selector, alice, address(originARM), DEFAULT_AMOUNT), + abi.encode(false) + ); + + vm.prank(alice); + vm.expectRevert(_safeERC20FailedOperation(address(weth))); + originARM.deposit(DEFAULT_AMOUNT); + + assertEq(originARM.balanceOf(alice), 0, "shares minted"); + } + + function test_RevertWhen_SwapExactTokensForTokens_InputTransferFromReturnsFalse() public { + uint256 amountIn = DEFAULT_AMOUNT; + deal(address(oeth), alice, amountIn); + deal(address(weth), address(originARM), DEFAULT_AMOUNT); + + vm.prank(alice); + oeth.approve(address(originARM), amountIn); + + vm.mockCall( + address(oeth), + abi.encodeWithSelector(IERC20.transferFrom.selector, alice, address(originARM), amountIn), + abi.encode(false) + ); + + vm.prank(alice); + vm.expectRevert(_safeERC20FailedOperation(address(oeth))); + originARM.swapExactTokensForTokens(oeth, weth, amountIn, 0, alice); + } + + function test_RevertWhen_SwapExactTokensForTokens_OutputTransferReturnsFalse() public { + uint256 amountIn = DEFAULT_AMOUNT; + uint256 amountOut = amountIn * _buyPrice(address(oeth)) / originARM.PRICE_SCALE(); + deal(address(oeth), alice, amountIn); + deal(address(weth), address(originARM), amountOut); + + vm.prank(alice); + oeth.approve(address(originARM), amountIn); + + vm.mockCall( + address(weth), abi.encodeWithSelector(IERC20.transfer.selector, alice, amountOut), abi.encode(false) + ); + + vm.prank(alice); + vm.expectRevert(_safeERC20FailedOperation(address(weth))); + originARM.swapExactTokensForTokens(oeth, weth, amountIn, 0, alice); + } + + function test_RevertWhen_SwapTokensForExactTokens_InputTransferFromReturnsFalse() public { + uint256 amountOut = DEFAULT_AMOUNT / 2; + uint256 amountIn = amountOut * originARM.PRICE_SCALE() / _buyPrice(address(oeth)) + 3; + deal(address(oeth), alice, amountIn); + deal(address(weth), address(originARM), amountOut); + + vm.prank(alice); + oeth.approve(address(originARM), amountIn); + + vm.mockCall( + address(oeth), + abi.encodeWithSelector(IERC20.transferFrom.selector, alice, address(originARM), amountIn), + abi.encode(false) + ); + + vm.prank(alice); + vm.expectRevert(_safeERC20FailedOperation(address(oeth))); + originARM.swapTokensForExactTokens(oeth, weth, amountOut, type(uint256).max, alice); + } + + function test_RevertWhen_SwapTokensForExactTokens_OutputTransferReturnsFalse() public { + uint256 amountOut = DEFAULT_AMOUNT / 2; + uint256 amountIn = amountOut * originARM.PRICE_SCALE() / _buyPrice(address(oeth)) + 3; + deal(address(oeth), alice, amountIn); + deal(address(weth), address(originARM), amountOut); + + vm.prank(alice); + oeth.approve(address(originARM), amountIn); + + vm.mockCall( + address(weth), abi.encodeWithSelector(IERC20.transfer.selector, alice, amountOut), abi.encode(false) + ); + + vm.prank(alice); + vm.expectRevert(_safeERC20FailedOperation(address(weth))); + originARM.swapTokensForExactTokens(oeth, weth, amountOut, type(uint256).max, alice); + } + + function test_RevertWhen_ClaimRedeem_TransferReturnsFalse() public deposit(alice, DEFAULT_AMOUNT) { + uint256 shares = originARM.balanceOf(alice); + + vm.prank(alice); + (uint256 requestId,) = originARM.requestRedeem(shares); + + vm.warp(block.timestamp + CLAIM_DELAY); + vm.mockCall( + address(weth), abi.encodeWithSelector(IERC20.transfer.selector, alice, DEFAULT_AMOUNT), abi.encode(false) + ); + + vm.prank(alice); + vm.expectRevert(_safeERC20FailedOperation(address(weth))); + originARM.claimRedeem(requestId); + } + + function test_RevertWhen_CollectFees_TransferReturnsFalse() public { + deal(address(weth), address(originARM), DEFAULT_AMOUNT); + deal(address(oeth), bob, DEFAULT_AMOUNT); + + vm.startPrank(bob); + oeth.approve(address(originARM), DEFAULT_AMOUNT); + originARM.swapExactTokensForTokens(oeth, weth, DEFAULT_AMOUNT, 0, bob); + vm.stopPrank(); + + uint256 fees = originARM.feesAccrued(); + vm.mockCall( + address(weth), abi.encodeWithSelector(IERC20.transfer.selector, feeCollector, fees), abi.encode(false) + ); + + vm.expectRevert(_safeERC20FailedOperation(address(weth))); + originARM.collectFees(); + } + + function test_RevertWhen_AddBaseAsset_ApproveReturnsFalse() public { + MockERC20 newBaseAsset = new MockERC20("New Base", "NEW", 18); + OriginAssetAdapter adapter = + new OriginAssetAdapter(address(originARM), address(newBaseAsset), address(weth), address(vault)); + + vm.mockCall( + address(newBaseAsset), + abi.encodeWithSelector(IERC20.approve.selector, address(adapter), type(uint256).max), + abi.encode(false) + ); + + vm.prank(governor); + vm.expectRevert(_safeERC20FailedOperation(address(newBaseAsset))); + originARM.addBaseAsset( + address(newBaseAsset), + address(adapter), + 992 * 1e33, + 1001 * 1e33, + type(uint128).max, + type(uint128).max, + 1e36, + true + ); + } + + function test_RevertWhen_Allocate_ApproveReturnsFalse() public { + uint256 depositAmount = 2 ether; + address[] memory markets = new address[](1); + markets[0] = address(market); + + vm.prank(governor); + originARM.addMarkets(markets); + + deal(address(weth), address(originARM), depositAmount); + vm.mockCall( + address(weth), + abi.encodeWithSelector(IERC20.approve.selector, address(market), depositAmount), + abi.encode(false) + ); + + vm.prank(governor); + vm.expectRevert(_safeERC20FailedOperation(address(weth))); + originARM.setActiveMarket(address(market)); + } + + function test_RevertWhen_AdapterRequestRedeem_TransferFromReturnsFalse() public { + uint256 shares = DEFAULT_AMOUNT; + deal(address(oeth), address(originARM), shares); + + vm.mockCall( + address(oeth), + abi.encodeWithSelector(IERC20.transferFrom.selector, address(originARM), address(originAssetAdapter), shares), + abi.encode(false) + ); + + vm.prank(governor); + vm.expectRevert(_safeERC20FailedOperation(address(oeth))); + originARM.requestBaseAssetRedeem(address(oeth), shares); + } + + function test_RevertWhen_AdapterRedeem_TransferReturnsFalse() public { + uint256 shares = DEFAULT_AMOUNT; + deal(address(oeth), address(originARM), shares); + + vm.prank(governor); + originARM.requestBaseAssetRedeem(address(oeth), shares); + + deal(address(weth), address(vault), shares); + vm.mockCall( + address(weth), + abi.encodeWithSelector(IERC20.transfer.selector, address(originARM), shares), + abi.encode(false) + ); + + vm.prank(governor); + vm.expectRevert(_safeERC20FailedOperation(address(weth))); + originARM.claimBaseAssetRedeem(address(oeth), shares); + } + + function test_RevertWhen_AdapterConstructor_ApproveReturnsFalse() public { + MockFalseReturnERC20 falseToken = new MockFalseReturnERC20("False", "FALSE", 18); + falseToken.setApproveReturnsFalse(true); + + vm.expectRevert(_safeERC20FailedOperation(address(falseToken))); + new OriginAssetAdapter(address(originARM), address(falseToken), address(weth), address(vault)); + } + + function test_RevertWhen_AdapterInitialize_ApproveReturnsFalse() public { + MockFalseReturnERC20 falseToken = new MockFalseReturnERC20("False", "FALSE", 18); + OriginAssetAdapter adapter = + new OriginAssetAdapter(address(originARM), address(falseToken), address(weth), address(vault)); + falseToken.setApproveReturnsFalse(true); + + vm.expectRevert(_safeERC20FailedOperation(address(falseToken))); + adapter.initialize(); + } + + function _buyPrice(address asset) internal view returns (uint256 buyPrice) { + (uint128 buyPriceMem,,,,,,,) = originARM.baseAssetConfigs(asset); + buyPrice = buyPriceMem; + } + + function _safeERC20FailedOperation(address token) internal pure returns (bytes memory) { + return abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, token); + } +} diff --git a/test/unit/mocks/MockFalseReturnERC20.sol b/test/unit/mocks/MockFalseReturnERC20.sol new file mode 100644 index 00000000..76383fe5 --- /dev/null +++ b/test/unit/mocks/MockFalseReturnERC20.sol @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.23; + +import {MockERC20} from "@solmate/test/utils/mocks/MockERC20.sol"; + +contract MockFalseReturnERC20 is MockERC20 { + bool public transferReturnsFalse; + bool public transferFromReturnsFalse; + bool public approveReturnsFalse; + + constructor(string memory name, string memory symbol, uint8 decimals) MockERC20(name, symbol, decimals) {} + + function setTransferReturnsFalse(bool value) external { + transferReturnsFalse = value; + } + + function setTransferFromReturnsFalse(bool value) external { + transferFromReturnsFalse = value; + } + + function setApproveReturnsFalse(bool value) external { + approveReturnsFalse = value; + } + + function approve(address spender, uint256 amount) public override returns (bool) { + if (approveReturnsFalse) return false; + return super.approve(spender, amount); + } + + function transfer(address to, uint256 amount) public override returns (bool) { + if (transferReturnsFalse) return false; + return super.transfer(to, amount); + } + + function transferFrom(address from, address to, uint256 amount) public override returns (bool) { + if (transferFromReturnsFalse) return false; + return super.transferFrom(from, to, amount); + } +}