code-423n4 / 2024-04-revert-mitigation-findings

1 stars 1 forks source link

M-20 MitigationConfirmed #21

Open c4-bot-6 opened 4 months ago

c4-bot-6 commented 4 months ago

Lines of code

Vulnerability details

C4 issue

M-20: Tokens can't be removed as a collateral without breaking liquidations and other core functions

Comment

In the original implementation, once a token is un-whitelisted by setting collateralFactorX32 =0, all the loans with this token as collateral cannot be liquidated due to division by zero in function _calculateLiquidation (with collateralValue = 0):

function _calculateLiquidation(uint256 debt, uint256 fullValue, uint256 collateralValue)
        internal
        pure
        returns (uint256 liquidationValue, uint256 liquidatorCost, uint256 reserveCost)
    {
        ...
        if (fullValue >= maxPenaltyValue) {
            // position value when position started to be liquidatable
            uint256 startLiquidationValue = debt * fullValue / collateralValue;
            ...
        }
}

Mitigation

PR #25 The mitigated code now only use collateralValue for calculation if it's > 0:

if (fullValue >= maxPenaltyValue) {
            if (collateralValue != 0) {
                // position value when position started to be liquidatable
                uint256 startLiquidationValue = debt * fullValue / collateralValue;
                uint256 penaltyFractionX96 =
                    (Q96 - ((fullValue - maxPenaltyValue) * Q96 / (startLiquidationValue - maxPenaltyValue)));
                uint256 penaltyX32 = MIN_LIQUIDATION_PENALTY_X32
                    + (MAX_LIQUIDATION_PENALTY_X32 - MIN_LIQUIDATION_PENALTY_X32) * penaltyFractionX96 / Q96;

                liquidationValue = debt * (Q32 + penaltyX32) / Q32;
            } else {
                liquidationValue = debt * (Q32 + MAX_LIQUIDATION_PENALTY_X32) / Q32;
            }
        }

If collateralValue = 0, then liquidationValue is fixed as 110% debt. The mitigation solved the original issue

Conclusion

LGTM

c4-judge commented 4 months ago

jhsagd76 marked the issue as satisfactory