claimUnassignedAssets is increasing debt but not checking for it in _finaliseTrove, opening up for self-liquidations

criticalRecon Audits

Impact

claimUnassignedAssets will increase the amount of debt and collateral to a trove by a certain amount

Resulting in a Coll Increase and a Debt Increase

https://github.com/blkswnStudio/ap/blob/8fab2b32b4f55efd92819bd1d0da9bed4b339e87/packages/contracts/contracts/BorrowerOperations.sol#L614-L665

solidity📋
function claimUnassignedAssets(
    uint _percentage,
    address _upperHint,
    address _lowerHint,
    bytes[] memory _priceUpdateData
  ) external payable override {
    if (_percentage == 0) revert ZeroDebtChange();
    /// @audit Self Liquidation Risk?
    address borrower = msg.sender;
    (ContractsCache memory contractsCache, LocalVariables_adjustTrove memory vars) = _prepareTroveAdjustment(
      borrower,
      _priceUpdateData,
      false
    );

    // handle debts
    DebtTokenAmount[] memory debtsToAdd = new DebtTokenAmount[](vars.priceCache.debtPrices.length);
    for (uint i = 0; i < vars.priceCache.debtPrices.length; i++) {
      address debtToken = vars.priceCache.debtPrices[i].tokenAddress;

      uint unassignedDebt = contractsCache.storagePool.getValue(debtToken, false, PoolType.Unassigned);
      if (unassignedDebt == 0) continue;

      uint toClaim = (unassignedDebt * _percentage) / DECIMAL_PRECISION;
      if (unassignedDebt == 0) continue;

      contractsCache.storagePool.transferBetweenTypes(debtToken, false, PoolType.Unassigned, PoolType.Active, toClaim);
      debtsToAdd[i] = DebtTokenAmount(IDebtToken(debtToken), toClaim, 0);
    }
    vars.newCompositeDebtInUSD += _getCompositeDebt(contractsCache.priceFeed, vars.priceCache, debtsToAdd);
    contractsCache.troveManager.increaseTroveDebt(borrower, debtsToAdd);

    // handle colls
    TokenAmount[] memory collsToAdd = new TokenAmount[](vars.priceCache.collPrices.length);
    for (uint i = 0; i < vars.priceCache.collPrices.length; i++) {
      address collToken = vars.priceCache.collPrices[i].tokenAddress;

      uint unassignedColl = contractsCache.storagePool.getValue(collToken, true, PoolType.Unassigned);
      if (unassignedColl == 0) continue;

      uint toClaim = (unassignedColl * _percentage) / DECIMAL_PRECISION;
      if (unassignedColl == 0) continue;

      contractsCache.storagePool.transferBetweenTypes(collToken, true, PoolType.Unassigned, PoolType.Active, toClaim);
      collsToAdd[i] = TokenAmount(collToken, toClaim);
    }
    vars.newCompositeCollInUSD += _getCompositeColl(contractsCache.priceFeed, vars.priceCache, collsToAdd);
    vars.newIMCR = _calculateIMCR(vars, collsToAdd, true);
    contractsCache.troveManager.increaseTroveColl(borrower, collsToAdd);

    _finaliseTrove(false, false, contractsCache, vars, borrower, _upperHint, _lowerHint);
  }

but it's calling _finaliseTrove(false, false, which means that these checks applied when _isDebtIncrease == true are going to be skipped

https://github.com/blkswnStudio/ap/blob/8fab2b32b4f55efd92819bd1d0da9bed4b339e87/packages/contracts/contracts/BorrowerOperations.sol#L978-L986

solidity📋
    if (_vars.isInRecoveryMode) {
      // BorrowerOps: Collateral withdrawal not permitted Recovery Mode
      if (_isCollWithdrawal) revert CollWithdrawPermittedInRM();
      if (_isDebtIncrease) _requireICRisAboveCCR(_vars.newICR);
    } else {
      // if Normal Mode

      // check if the individual minimum collateral ratio is met (based on the used coll types)
      if (_isCollWithdrawal || _isDebtIncrease) _requireICRisAboveIMCR(_vars.newICR, _vars.newIMCR);

Mitigation

Change the code to

solidity📋
_finaliseTrove(false, true, contractsCache, vars, borrower, _upperHint, _lowerHint);