Stormy - Liquidating underwater Cdp in the function batchLiquidateCdps doesn’t redistribute the bad debt accordingly among all other Cdps in the batch list.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/LiquidationLibrary.sol#L679-L727

Vulnerability details

Impact

Liquidating underwater Cdp in the function batchLiquidateCdps doesn’t redistribute the bad debt accordingly among all other Cdps in the batch list, which results to unfair redistribution of bad debt among all the positions left in the system.

Proof of Concept

The function batchLiquidateCdps is used to liquidate a list of Cdp positions, when the function is called the system liquidates the positions one by one and records the values of:

• the debt that needs to be burned
• the collateral that needs to be send to the liquidator
• the bad debt to redistribute if any
• the collateral reward
• the surplus collateral if any

Basically the system closes all Cdp positions and records the liquidation values which needs to be finalized in order for the liquidation to be successful. The problem here is that this values are finalized at the end of the function when the positions are already closed and all liquidations are already done, as a result if there is bad debt that needs to be redistributed it won't actually affect the Cdps in the batch list, but it will be compensated by all the remaining Cdp positions in the system. This issue leads to unfair redistribution of bad debt when the function batchLiquidateCdps is used to liquidate underwater Cdp.

    function batchLiquidateCdps(bytes32[] memory _cdpArray) external nonReentrantSelfAndBOps {
        require(
            _cdpArray.length != 0,
            "LiquidationLibrary: Calldata address array must not be empty"
        );

        LocalVariables_OuterLiquidationFunction memory vars;
        LiquidationTotals memory totals;

        // taking fee to avoid accounted for the calculation of the TCR
        _syncGlobalAccounting();

        vars.price = priceFeed.fetchPrice();
        (uint256 _TCR, uint256 systemColl, uint256 systemDebt) = _getTCRWithSystemDebtAndCollShares(
            vars.price
        );
        vars.recoveryModeAtStart = _TCR < CCR ? true : false;

        // Perform the appropriate batch liquidation - tally values and obtain their totals.
        if (vars.recoveryModeAtStart) {
            totals = _getTotalFromBatchLiquidate_RecoveryMode(
                vars.price,
                systemColl,
                systemDebt,
                _cdpArray
            );
        } else {
            //  if !vars.recoveryModeAtStart
            totals = _getTotalsFromBatchLiquidate_NormalMode(vars.price, _TCR, _cdpArray);
        }

        require(totals.totalDebtInSequence > 0, "LiquidationLibrary: nothing to liquidate");

        // housekeeping leftover collateral for liquidated CDPs
        if (totals.totalCollSurplus > 0) {
            activePool.transferSystemCollShares(address(collSurplusPool), totals.totalCollSurplus);
        }

        _finalizeLiquidation(
            totals.totalDebtToBurn,
            totals.totalCollToSendToLiquidator,
            totals.totalDebtToRedistribute,
            totals.totalCollReward,
            totals.totalCollSurplus,
            systemColl,
            systemDebt,
            vars.price
        );
    }

Tools Used

Manual review.

Recommended Mitigation Steps

The system should finalize the liquidation values after closing every position, by doing this if there is any bad debt the remaining Cdp positions in the batch list will accordingly take the earned bad debt they need to pay and unfair redistribution won't happen.

Assessed type

Other

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-pre-sort]

bytes032 marked the issue as duplicate of #36

[c4-judge]

jhsagd76 marked the issue as satisfactory