Dancing Honey Orangutan
High
There is no way to set feeRecipient
in Leverager
contract. This will lead to liquidation failure because transfer to empty address is not allowed in most ERC20 tokens.
In Leverager.sol
, feeRecipient
address is not initialized in the constructor and does not have setter method either.
So when liquidation fee is claimed, Leverager
contract will try to send tokens to empty address address(0)
:
@> if (pf0 > 0) IERC20(up.token0).safeTransfer(feeRecipient, pf0);
@> if (pf1 > 0) IERC20(up.token1).safeTransfer(feeRecipient, pf1);
This will revert in most cases as any sane ERC20 tokens will prevent transfer to empty address.
- Liquidator tries to liquidate a position that has total value USD is greater than borrowed value USD
N/A
N/A
This will lead to liquidation DoS, if transferred token prevents transferring to empty address. If the token allows transferring to empty address, liquidation fee is lost forever.
Apply the following patch:
diff --git a/yieldoor/test/Leverager.t.sol b/yieldoor/test/Leverager.t.sol
index c8d7a38..3bcaebc 100644
--- a/yieldoor/test/Leverager.t.sol
+++ b/yieldoor/test/Leverager.t.sol
@@ -21,8 +21,10 @@ import {PriceFeed} from "../src/PriceFeed.sol";
import {LendingPool} from "../src/LendingPool.sol";
import {MockOracle} from "./utils/MockOracle.sol";
import {DataTypes} from "../src/types/DataTypes.sol";
+import {SafeERC20} from "@openzeppelin/token/ERC20/utils/SafeERC20.sol";
contract LeveragerTest is BaseTest {
+ using SafeERC20 for IERC20;
address leverager;
address lendingPool;
address priceFeed;
@@ -95,6 +97,19 @@ contract LeveragerTest is BaseTest {
assertEq(borrowingRate, 0);
}
+ function test_transferToEmptyAddress() external {
+ deal(address(weth), leverager, 1e18);
+ deal(address(wbtc), leverager, 1e8);
+ vm.startPrank(leverager);
+ // fee recipient is address(0)
+ assertEq(ILeverager(leverager).feeRecipient(), address(0));
+ // weth allows transfer to address(0), in this case liquidation fee is lost forever
+ weth.safeTransfer(ILeverager(leverager).feeRecipient(), 0.1e18);
+ // wbtc does not allow transfer to address(0), in this case liquidation will revert
+ wbtc.safeTransfer(ILeverager(leverager).feeRecipient(), 0.1e8);
+ vm.stopPrank();
+ }
+
function test_cantOpenPositionWhenReserveInactive() external {
uint256 borrowingRate = ILendingPool(lendingPool).borrowingRateOfReserve(address(wbtc));
assertEq(borrowingRate, 0);
Run the following command:
forge test --rpc-url $MAINNET_FORK_URL --fork-block-number 21926708 --match-test test_transferToEmptyAddress -vvv
Implement a setter function for feeRecipient
:
diff --git a/yieldoor/src/Leverager.sol b/yieldoor/src/Leverager.sol
index b59c56c..ad20c7a 100644
--- a/yieldoor/src/Leverager.sol
+++ b/yieldoor/src/Leverager.sol
@@ -317,7 +317,7 @@ contract Leverager is ReentrancyGuard, Ownable, ERC721, ILeverager {
uint256 bPrice = IPriceFeed(pricefeed).getPrice(up.denomination);
uint256 borrowedValue = owedAmount * bPrice / ERC20(up.denomination).decimals();
- if (totalValueUSD > borrowedValue) {
+ if (totalValueUSD > borrowedValue && feeRecipient != address(0) && liquidationFee != 0) {
// What % of the amountsOut are profit is calculated by `(totalValueUSD - borrowedUSD) / totalValueUSD`
// Then, on top of that, we calculate the protocol fee and scale it in 1e18.
@@ -577,4 +577,11 @@ contract Leverager is ReentrancyGuard, Ownable, ERC721, ILeverager {
Position memory pos = positions[_id];
return pos;
}
+
+ function setFeeRecipient(address _feeRecipient) external onlyOwner {
+ feeRecipient = _feeRecipient;
+ if (feeRecipient == address(0)) {
+ require(liquidationFee == 0, "Liquidation fee should be 0 when fee recipient is empty");
+ }
+ }
}