When liquidate bad loan, protocol does not pay out of reserve as intended and only pay out of lender pocket. Missing update exchangeRate after bad loan calculation.

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L1123-L1141

Vulnerability details

Quoting comment from V3Vault.sol on how liquidation work:

// calculates amount which needs to be payed to liquidate position // if position is too valuable - not all of the position is liquididated - only needed amount // if position is not valuable enough - missing part is covered by reserves - if not enough reserves - collectively by other borrowers

This part if position is not valuable enough - missing part is covered by reserves is not true. When bad loan happen and collateral not enough to repay debt (original borrow + interest).

The debt have never been 100% repaid or write off. Exchange rate (borrowIndex,SupplyIndex) stay the same. The pool still consider debt have been fully repaid while no USDC have been deposited to the pool or taken from somewhere to cover entire 100% debt.

Impact

Wrong handling of liquidation of really bad loan. A chunk of debt never repaid and no money taken out of anywhere to compensate for it and no exchange rate have been updated to reflect this.

Causing wrong calculation for global debt, exchange rate. This mean not all future lenders can take out their correct share worth in USDC.

Proof of Concept

With current implementation of V3Vault.sol. Here is simplified stuff happen when calculate liquidation of bad loan:

• Check loan is healthy or not
• calculate how much need to be liquidated
• how much USDC token need to repay debt
• how much portion of NFT liquidated to send to liquidator
• how much portion of NFT liquidated to send to original borrower
• how much USDC token need to take out of pool to compensate if loan is really bad.

When collateral is worth more than debt

Liquidator pay full debt and receive ~110% value of debt in collateral. https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L1103-L1111

When collateral is worth less than debt

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L1112-L1118

    // all position value. //@fullValue is just NFT collateral worth in USDC
    liquidationValue = fullValue; //@ fullvalue < maxPenaltyValue < debt * 110%

    uint256 penaltyValue = fullValue * (Q32 - MAX_LIQUIDATION_PENALTY_X32) / Q32;//@full value * 90% 
    liquidatorCost = penaltyValue;
    reserveCost = debt - penaltyValue;//penaltyValue = fullValue * 90%.         

Liquidator will pay 90% of NFT value in USDC and get whole NFT positions in return. The rest of debt suppose to be taken from reserve.

Taking money out of reserve function here:

    // calculates if there are enough reserves to cover liquidaton - if not its shared between lenders
    function _handleReserveLiquidation(
        uint256 reserveCost,
        uint256 newDebtExchangeRateX96,
        uint256 newLendExchangeRateX96
    ) internal returns (uint256 missing) {
        (,, uint256 reserves) = _getAvailableBalance(newDebtExchangeRateX96, newLendExchangeRateX96);
        //@note reserve is total expected portfiolo worth include debt not mature yet. reserve= current cash money + debt worth - cash give to lending 
        // if not enough - democratize debt
        if (reserveCost > reserves) {
            missing = reserveCost - reserves;

            uint256 totalLent = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up);

            // this lines distribute missing amount and remove it from all lent amount proportionally
            newLendExchangeRateX96 = (totalLent - missing) * newLendExchangeRateX96 / totalLent;
            lastLendExchangeRateX96 = newLendExchangeRateX96;//@note reduce lendExchangeRate or SupplyRate after liquidite bad loan. If money come out from pool, reduce this indexRate
            emit ExchangeRateUpdate(newDebtExchangeRateX96, newLendExchangeRateX96);
        }//@audit handling bad loan. liquidation need to shave off exchangerate from both supplyRate and borrowRate
    }//@audit M handling badloan from reserve does not remove money from reserve.

No debt is write off and no money is taken out of reserve

Look at this condition if (reserveCost > reserves) this never be true in practice. Here is simplified math on how Reserve is calculated Reserve = (totalDebt + Debt interest) - (totalLend + Lend interest) + USDC cash in pool

Obviously, 1 small bad NFT loan rarely happen when reserve is enough to cover it.

But there is no else-case to handle when reserve is enough to cover bad loan. This causing these problems with calculation:

• The lastDebtExchangeRateX96 or debt exchange Rate does not change. This mean all debt still consider fully repaid. This include original borrow + interest.
• These USDC token suppose to be taken out from the pool/protocol reserve or atleast from magic place. But it is not.
• lastLendExchangeRateX96 keep the same meaning Protocol expect to have full debt + interest repaid to lenders. But 1 failed loan already happen with no USDC provided. This mean there are less USDC token than lenders expect to take out from the pool.

There exist a Protocol Reserve that was not tracked

But there exist a reserve belong to pool/protocol that can be reduced here

    // percentage of interest which is kept in the protocol for reserves
    uint32 public reserveFactorX32 = 0;

We can see this reserve taken from user profit but this reserve value is not tracked.

        supplyRateX96 = supplyRateX96.mulDiv(Q32 - reserveFactorX32, Q32);

        // always growing or equal
        uint256 lastRateUpdate = lastExchangeRateUpdate;

        if (lastRateUpdate > 0) {
            newDebtExchangeRateX96 = oldDebtExchangeRateX96
                + oldDebtExchangeRateX96 * (block.timestamp - lastRateUpdate) * borrowRateX96 / Q96;
            newLendExchangeRateX96 = oldLendExchangeRateX96
                + oldLendExchangeRateX96 * (block.timestamp - lastRateUpdate) * supplyRateX96 / Q96;
        } else {
            newDebtExchangeRateX96 = oldDebtExchangeRateX96;
            newLendExchangeRateX96 = oldLendExchangeRateX96;
        }

User receive less profit by reducing supplyRateX96. Hence lastLendExchangeRateX96 will be smaller than it should be. This mean user receive less profit than they should be.

The reduced profit is now part of reserve, belong to the pool, and no one can access this except admin through withdrawReserves() function.

Because vault does how much lastLendExchangeRateX96 rate it have already reduced through reserveFactorX32, it is impossible to keep track of how much reserve can be reduced.

Thats why _handleReserveLiquidation() does not remove money from reserve. It is impossible to know how much reserve can be reduced.

Tools Used

manual

Recommended Mitigation Steps

To find missing USDC token to repay bad loan.

• Protocol can take it from reserveFactorX32 directly. By tracking how much lastLendExchangeRateX96 have been reduced by reserveFactorX32 and take it out from the pool. This is unlikely and unpractical.
• Instead of reducing lender assets through lastLendExchangeRateX96. Protocol can also reduce global debt directly. Reduce future profit to compensate for bad loan. This mean decrease lastDebtExchangeRateX96 by missing debt amount. Borrower now have to pay less repayment.

    // calculates if there are enough reserves to cover liquidaton - if not its shared between lenders
    function _handleReserveLiquidation(
        uint256 reserveCost,
        uint256 newDebtExchangeRateX96,
        uint256 newLendExchangeRateX96
    ) internal returns (uint256 missing) {
        (,, uint256 reserves) = _getAvailableBalance(newDebtExchangeRateX96, newLendExchangeRateX96);

        // if not enough - democratize debt
        if (reserveCost > reserves) {
            missing = reserveCost - reserves;

            uint256 totalLent = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up);

            // this lines distribute missing amount and remove it from all lent amount proportionally
            newLendExchangeRateX96 = (totalLent - missing) * newLendExchangeRateX96 / totalLent;
            lastLendExchangeRateX96 = newLendExchangeRateX96;
            emit ExchangeRateUpdate(newDebtExchangeRateX96, newLendExchangeRateX96);
        }
        else {//@audit introduce else case solution instead of praying there is enough liquidity and no one notice bad debt
            // reduce global debt directly
            uint totalDebt = _convertToAssets(debtSharesTotal,newDebtExchangeRateX96 , Math.Rounding.Up);
            newDebtExchangeRateX96 = (totalDebt - reserveCost) * newDebtExchangeRateX96 / totalDebt;
            lastDebtExchangeRateX96 = newDebtExchangeRateX96;
            emit ExchangeRateUpdate(newDebtExchangeRateX96, newLendExchangeRateX96);
        }
    }

Assessed type

Math

[c4-pre-sort]

0xEVom marked the issue as sufficient quality report

[kalinbas]

It is not "taken out of anywhere".

Reserves are defined as balance + debt - lent

So in a liquidation with reserves what happens is the following:

• The liquidator pays for the (insufficient) value of collateral - discount
• The collateral tokens are sent to liquidator
• We know that the missing part to repay the debt is less or equal to the reserves (so by just cleaning up the loan and removing the debt) it is implicitly removed from the reserves

[c4-sponsor]

kalinbas marked the issue as disagree with severity

[c4-sponsor]

kalinbas marked the issue as agree with severity

[c4-sponsor]

kalinbas (sponsor) disputed

[kalinbas]

What is true in the report is the fact that "lastLendExchangeRateX96" is not properly updated (which is a duplicate of issue #206

[c4-judge]

jhsagd76 marked the issue as duplicate of #206

[c4-judge]

jhsagd76 marked the issue as satisfactory

[c4-judge]

jhsagd76 changed the severity to 2 (Med Risk)

[c4-judge]

jhsagd76 marked the issue as unsatisfactory: Invalid