Description

The Auctions contract handles the liquidation of debtors. The groupedLiquidation function is used by the owner of the contract to liquidate multiple debtors at once. However, it does not check that the debtors are auctionable, which means that if the owner of the contract by mistake passes a nonauctionable debtor, the liquidation will still be performed, and the debtors will be left in an inconsistent state.

function groupedLiquidation(
    address market,
    address[] memory debtors
) external returns (uint256 liquidated, uint256 tip) {

    require(
        (_msgSender() == owner()) || // owner
            attributes[_msgSender()].has(Role.EXECUTE_AUCTION),
        "authorizer"
    );
    (liquidated, tip) = IMarket(market).credit().liquidate(
        debtors,
        _msgSender()
    );
    emit GroupedLiquidations(
        market,
        block.timestamp,
        debtors,
        liquidated,
        tip,
        _msgSender()
    );
}

Impact

Since this function is a privileged function, the impact is limited to the owner of the contract. However, it can still lead to an inconsistent state of the debtors, which can lead to further unexpected behavior.

Recommendations

We recommend individually checking that each debtor is auctionable before performing the grouped liquidation. For example,

function groupedLiquidation(
    address market,
    address[] memory debtors
) external returns (uint256 liquidated, uint256 tip) {

+   for(uint256 i = 0; i < debtors.length; i++) {
+       require(isAuctionable(market, debtors[i]), "not auctionable");
+   }
    // ...
}

Remediation

The Nukem team fixed this finding in commit 571dbc66↗ by ensuring every debtor is auctionable.