J4de - The `BlueBerryBank.sol#getPositionRisk` function calculates the risk unreasonably

[sherlock-admin]

J4de

high

The BlueBerryBank.sol#getPositionRisk function calculates the risk unreasonably

Summary

The BlueBerryBank.sol#getPositionRisk function calculates the risk unreasonably

Vulnerability Detail

File: BlueBerryBank.sol
456     function getPositionRisk(
457         uint256 positionId
458     ) public view returns (uint256 risk) {
459         uint256 pv = getPositionValue(positionId);
460         uint256 ov = getDebtValue(positionId);
461         uint256 cv = getIsolatedCollateralValue(positionId);
462
463         if (
464             (cv == 0 && pv == 0 && ov == 0) || pv >= ov // Closed position or Overcollateralized position
465         ) {
466             risk = 0;
467         } else if (cv == 0) {
468             // Sth bad happened to isolated underlying token
469             risk = Constants.DENOMINATOR;
470         } else {
471             risk = ((ov - pv) * Constants.DENOMINATOR) / cv;
472         }
473     }

GetPositionRisk function is affected by three factors when calculating the risk, namely pv (PositionValue), ov (DebtValue) and cv (IsolatedCollateralValue). ov is the amount of debt, pv and cv together are the amount of collateral. Therefore, the risk rate is calculated as ov / (pv + cv).

The problem now is that as long as pv > ov in the getPositionRisk function, the risk is considered to be 0. This is obviously unreasonable, for example:

• ov = 90
• pv = 100
• cv = 0

In this case, the risk of this position is already very high (90% of the collateral value has been borrowed), but the risk calculated by the getPositionRisk function is still 0. And no one can liquidate the position. At this time, if the price of the collateral falls, the pv will drop:

• ov = 90
• pv = 80 ↓
• cv = 0

At this time, the position is obviously insolvent, but it will still not be liquidated (even if the position status is liquidable), because the liquidator will not be willing to suffer this loss.

Impact

As long as the user's cv is 0, it will never be liquidated.

1. pv >= ov, risk = 0
2. pv < ov, liquidation is not good for liquidators, no one wants to liquidate this position

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L456-L473

Tool used

Manual Review

Recommendation

    function getPositionRisk(
        uint256 positionId
    ) public view returns (uint256 risk) {
        uint256 pv = getPositionValue(positionId);
        uint256 ov = getDebtValue(positionId);
        uint256 cv = getIsolatedCollateralValue(positionId);

        if (
-           (cv == 0 && pv == 0 && ov == 0) || pv >= ov // Closed position or Overcollateralized position
+           cv == 0 && pv == 0 && ov == 0
        ) {
            risk = 0;
        } else if (cv == 0) {
            // Sth bad happened to isolated underlying token
            risk = Constants.DENOMINATOR;
        } else {
-           risk = ((ov - pv) * Constants.DENOMINATOR) / cv;
+           risk = (ov * Constants.DENOMINATOR) / (cv + pv);
        }
    }