Withdrawing uncollateralized deposits is possible even though the position is in liquidation mode

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseSecurity/WiseSecurity.sol#L263-L265 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseSecurity/WiseSecurity.sol#L125-L128

Vulnerability details

Impact

Users can withdraw uncollateralized deposits even though their position is liquidable, as opposed to the README, if the position is in liquidation mode, users should use their uncollateralized deposits to avoid liquidation instead of removing them

Proof of Concept

When withdrawing deposits from public pools, at the end of the tx is executed the WiseLending._healthStateCheck() function, which depending on the value of the powerFarmCheck will determine if the position's collateral is enough to cover the borrows.

• If powerFarmCheck is true, it will use the bare value of the collateral, meaning, the collateralFactor is not applied to the collateral's value
• If powerFarmCheck is false, it will use the weighted value of the collateral, meaning, the collateralFactor is applied to the collateral's value.

When withdrawing an uncollateralized deposit, the WiseCore._coreWithdrawToken() function calls the WiseSecurity.checksWithdraw() function to determine the value of the powerFarmCheck, if the pool from where the tokens are being withdrawn is uncollateralized, the powerFarmCheck will be set to true, which will cause that the WiseLending._healthStateCheck() function uses the bare value of the full collateral to determine if the collateral can cover the existing borrows.

• Using the bare value of the collateral does not accurately reflect if a position is liquidable or not, the liquidation's logic in the WiseSecurity.checksLiquidation() function uses the weightedCollateral instead of the bareCollateral to determine if the position is liquidable or not. So, this difference to determine if the position's collateral can cover the borrows when withdrawing uncollateralized deposits and when doing liquidation causes a discrepancy to allow the withdrawal of uncollateralized deposits even though the position is in liquidation mode.

For example, a user requests a withdrawal of an uncollateralized deposit in a position with the below values:

• 1.5e18 ETH of bare collateral
• 1.2e18 ETH of weightedCollateral
• 1.3e18 ETH of borrows
  • The withdrawal will be allowed even though the position is in liquidation mode. Because of the powerFarmCheck being set to true, the WiseLending._healthStateCheck() function will check if 95% of the bareCollateral can cover the borrows, 95% of 1.5e18 ETH would be 1.425e18 ETH, thus, the withdrawal will be possible, even though the position is in liquidation mode.

WiseSecurity.sol

function checksWithdraw(
...
)
...
{
...

if (_isUncollateralized(_nftId, _poolToken) == true) {
    return true;
}

...

}

function _getState( uint256 _nftId, bool _powerFarm ) internal view returns (bool) { ...

//@audit-info => If `powerFarmCheck` is true, overalCollateral will be computed using the value of the `bareCollateral`
//@audit-info => If `powerFarmCheck` is false, overalCollateral will be computed using the value of the `weightedCollateral`

uint256 overallCollateral = _powerFarm == true
    ? overallETHCollateralsBare(_nftId)
    : overallETHCollateralsWeighted(_nftId);

//@audit-info => If 95% of the overalCollateral > borrowAmount, withdrawal will be allowed!
return overallCollateral
    * BORROW_PERCENTAGE_CAP
    / PRECISION_FACTOR_E18
    < borrowAmount;

}

  - Now, when using the same values but for a liquidation, we have that the `weightedCollateral` is not enough to cover the borros, thus, the position is liquidable.

> WiseSecurity.sol

function checksLiquidation( ... ) external view { ...

//@audit-info => When doing liquidations, the value of the `weightedCollateral` is used to determine if the position is liquidable or not!
canLiquidate(
    borrowETHTotal,
    weightedCollateralETH
);

...

}

## Tools Used
Manual Audit

## Recommended Mitigation Steps
If the protocol wants to enforce that users use their uncollateralized deposits to avoid liquidations when the positions are liquidable, don't set to `true` the `powerFarmCheck` when doing withdrawals for uncollateralized deposits. Allow the [`WiseLending._healthStateCheck() function`](https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseLending.sol#L77-L90) to validate if the position is indeed in liquidation mode by using the `weightedCollateral` instead of the `bareCollateral` value.

> WiseSecurity.sol

function checksWithdraw( .. ) .. { ...

• if (_isUncollateralized(_nftId, _poolToken) == true) {
• return true;

• }

  ... }

Assessed type

Context

[c4-pre-sort]

GalloDaSballo marked the issue as sufficient quality report

[GalloDaSballo]

Would have been better to have a POC

[c4-pre-sort]

GalloDaSballo marked the issue as primary issue

[vonMangoldt]

Thats no issue since the additional check is reflecting a higher %. We will keep it like that

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 marked the issue as selected for report

[thebrittfactor]

For transparency and per conversation with the sponsors, see here for the Wise Lending team's mitigation.