Cuddly Ginger Tadpole - Fetching principal amount in active loans can revert if there were partial repayments previously

[sherlock-admin3]

Cuddly Ginger Tadpole

Medium

Fetching principal amount in active loans can revert if there were partial repayments previously

Summary

The issue shows how the value of the total principal amount outstanding in active loans can revert if there were previously partial repayments made by a borrower resulting in a value of the total principal tokens repaid exceeding the total principal tokens lent and therefore in an underflow. This situation occurs when liquidating a defaulted loan.

Root Cause

The root cause lies in the fact that repayments can be partial and the value of total principal tokens repaid can be increased via repay callback on these amounts. And when the loan is liquidated, this value is increased on the full principal amount of the loan again.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

The issue occurs when a borrower makes partial repayments and then the loan is defaulted creating a situation when totalPrincipalTokensRepaid > totalPrincipalTokensLended.

Impact

The crucial function getTotalPrincipalTokensOutstandingInActiveLoans() would become unusable.

PoC

Let's take a look at the current formula inside of the getTotalPrincipalTokensOutstandingInActiveLoans() function:

https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L712-718

   function getTotalPrincipalTokensOutstandingInActiveLoans()
        public
        view
        returns (uint256)
    {
        return totalPrincipalTokensLended - totalPrincipalTokensRepaid;
    }

totalPrincipalTokensLended is increased when the bid is accepted and the totalPrincipalTokensRepaid value is increased when there are repayments made to the loan or the loan is defaulted and liquidated:

https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L706-707

 //can use principal amt to increment amt paid back!! nice for math .
        totalPrincipalTokensRepaid += principalAmount;

Let's say there were 300 USDT lent to a borrower and there were several partial repayments made via TellerV2 contract resulting in a total amount repaid being 150 USDT. There is a callback that increases the principal amount:

https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L928-945

 function repayLoanCallback(
        uint256 _bidId,
        address repayer,
        uint256 principalAmount,
        uint256 interestAmount
    ) external onlyTellerV2 whenForwarderNotPaused whenNotPaused bidIsActiveForGroup(_bidId) { 
        totalPrincipalTokensRepaid += principalAmount;
        totalInterestCollected += interestAmount;

         emit LoanRepaid(
            _bidId,
            repayer,
            principalAmount,
            interestAmount,
            totalPrincipalTokensRepaid,
            totalInterestCollected
        );
    }

However, if a borrower fails to repay the remaining 150 USDT, the loan is defaulted and the liquidator would need to pay the principal amount according to this function:

https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L771-781

  function _getAmountOwedForBid(uint256 _bidId )
        internal
        view
        virtual
        returns (uint256 amountDue)
    {
        (,,,, amountDue, , ,  )
         = ITellerV2(TELLER_V2).getLoanSummary(_bidId);
       
    }

https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L1245-1269

 function getLoanSummary(uint256 _bidId)
        external
        view
        returns (
            address borrower,
            address lender,
            uint256 marketId,
            address principalTokenAddress,
            uint256 principalAmount,
            uint32 acceptedTimestamp,
            uint32 lastRepaidTimestamp,
            BidState bidState
        )
    {
        Bid storage bid = bids[_bidId];

        borrower = bid.borrower;
        lender = getLoanLender(_bidId);
        marketId = bid.marketplaceId;
        principalTokenAddress = address(bid.loanDetails.lendingToken);
        principalAmount = bid.loanDetails.principal;
        acceptedTimestamp = bid.loanDetails.acceptedTimestamp;
        lastRepaidTimestamp = V2Calculations.lastRepaidTimestamp(bids[_bidId]);
        bidState = bid.state;
    }

The principalAmount value here is the total principal amount of the vault (in our example, it's 300 USDT). This creates a situation where totalPrincipalTokensRepaid becomes 450 USDT and totalPrincipalTokensLended remains 300 USDT resulting in a function underflow and revert.

Mitigation

Make sure the totalPrincipalTokensRepaid is less than the amount lended by taking into account the possibility of the partial repayments.