Liquidation logic is incorrect in some conditions

[code423n4]

Lines of code

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L264-L294
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L532-L552

Vulnerability details

Impact

Because purchaseLiquidationAuctionNFT function clears remaining debt of debtor if he has no more collateral, it's possible that when 2 auctions exists in same time, liquidation logic will not work properly and debt will be nullified before last auction is finished.

Proof of Concept

When user has a bad debt, then anyone can start auction for his nft using startLiquidationAuction function. Also there is check that 2 days have passed since last auction for the debtor was created.
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L321-L323

        if (block.timestamp - info.latestAuctionStartTime < liquidationAuctionMinSpacing) {
            revert IPaprController.MinAuctionSpacing();
        }

The starting price for auction is 3*oraclePrice in papr. That means that usually noone will buy token at start time, they will be waiting when price will decrease. So it's actually possible that after 2 days the first auction still not finished and someone will start new one if debtor has another collateral token in vault.

To purchase token, liquidator can call purchaseLiquidationAuctionNFT function.
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L264-L294

    function purchaseLiquidationAuctionNFT(
        Auction calldata auction,
        uint256 maxPrice,
        address sendTo,
        ReservoirOracleUnderwriter.OracleInfo calldata oracleInfo
    ) external override {
        uint256 collateralValueCached = underwritePriceForCollateral(
            auction.auctionAssetContract, ReservoirOracleUnderwriter.PriceKind.TWAP, oracleInfo
        ) * _vaultInfo[auction.nftOwner][auction.auctionAssetContract].count;
        bool isLastCollateral = collateralValueCached == 0;


        uint256 debtCached = _vaultInfo[auction.nftOwner][auction.auctionAssetContract].debt;
        uint256 maxDebtCached = isLastCollateral ? debtCached : _maxDebt(collateralValueCached, updateTarget());
        /// anything above what is needed to bring this vault under maxDebt is considered excess
        uint256 neededToSaveVault = maxDebtCached > debtCached ? 0 : debtCached - maxDebtCached;
        uint256 price = _purchaseNFTAndUpdateVaultIfNeeded(auction, maxPrice, sendTo);
        uint256 excess = price > neededToSaveVault ? price - neededToSaveVault : 0;
        uint256 remaining;


        if (excess > 0) {
            remaining = _handleExcess(excess, neededToSaveVault, debtCached, auction);
        } else {
            _reduceDebt(auction.nftOwner, auction.auctionAssetContract, address(this), price);
            remaining = debtCached - price;
        }


        if (isLastCollateral && remaining != 0) {
            /// there will be debt left with no NFTs, set it to 0
            _reduceDebtWithoutBurn(auction.nftOwner, auction.auctionAssetContract, remaining);
        }
    }

What is interesting for us is this check bool isLastCollateral = collateralValueCached == 0;. Using this check protocol checks if debtor still has any collateral. It is needed to nullify debt in case if it's still remaining and there is no collateral anymore.

The problem here is that protocol doesn't handle situation when few auctions are in progress is same time.

Problem scenario.
1.Debtor has 2 nft as collateral and he has bad debt.
2.startLiquidationAuction is called for 1 nft.
3.after 2 days no one still bought that nft and startLiquidationAuction is called for 2 nft.
4.Now debtor doesn't have any collateral. Debtor calls purchaseLiquidationAuctionNFT for 1 nft(it's cheap now). isLastCollateral param will be 0 and that means that the debt will be cleared after the token purchase.
5.Debtor calls purchaseLiquidationAuctionNFT for 2 nft and because his debt is 0(was nullified), then full token price(minus fees) will be sent to him as excess value.
6.As result debtor received 2 nft back and also full token price for 1 nft. Protocol didn't receive debt repayment.

As you can see at a time when no more collateral tokens is left in debtor's vault then the first purchase of any auctioned token will fully clear the debt which will benefit debtor, and leaves protocol with unpaid debts.

Tools Used

VsCode

Recommended Mitigation Steps

In case if debtor has more auctions then do not clear his debt if he has no more collateral tokens.

[c4-judge]

trust1995 marked the issue as duplicate of #70

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 changed the severity to 2 (Med Risk)

[c4-judge]

trust1995 marked the issue as not a duplicate

[c4-judge]

trust1995 changed the severity to 3 (High Risk)

[c4-judge]

trust1995 marked the issue as duplicate of #97