MaslarovK.eth - Wrong accounting in the `LenderCommitmentGroup_Smart::liquidateDefaultedLoanWithIncentive`

[sherlock-admin3]

MaslarovK.eth

high

Wrong accounting in the LenderCommitmentGroup_Smart::liquidateDefaultedLoanWithIncentive

Summary

Wrong accounting in the LenderCommitmentGroup_Smart::liquidateDefaultedLoanWithIncentive when totalPrincipalTokensRepaid is increased.

Vulnerability Detail

There is a problem in the LenderCommitmentGroup_Smart::liquidateDefaultedLoanWithIncentive when _tokenAmountDifference <= 0 (or with other words - in the else statement).

function liquidateDefaultedLoanWithIncentive(
        uint256 _bidId,
        int256 _tokenAmountDifference
    ) public bidIsActiveForGroup(_bidId) {
        uint256 amountDue = getAmountOwedForBid(_bidId, false);

        uint256 loanDefaultedTimeStamp = ITellerV2(TELLER_V2)
            .getLoanDefaultTimestamp(_bidId);

        int256 minAmountDifference = getMinimumAmountDifferenceToCloseDefaultedLoan(
                amountDue,
                loanDefaultedTimeStamp
            );

        require(
            _tokenAmountDifference >= minAmountDifference,
            "Insufficient tokenAmountDifference"
        );

        if (_tokenAmountDifference > 0) {
            //this is used when the collateral value is higher than the principal (rare)
            //the loan will be completely made whole and our contract gets extra funds too
            uint256 tokensToTakeFromSender = abs(_tokenAmountDifference);

            IERC20(principalToken).transferFrom(
                msg.sender,
                address(this),
                amountDue + tokensToTakeFromSender
            );

            tokenDifferenceFromLiquidations += int256(tokensToTakeFromSender);

            totalPrincipalTokensRepaid += amountDue;
        } else {
            uint256 tokensToGiveToSender = abs(_tokenAmountDifference);

            IERC20(principalToken).transferFrom(
                msg.sender,
                address(this),
                amountDue - tokensToGiveToSender
            );

            tokenDifferenceFromLiquidations -= int256(tokensToGiveToSender);
            totalPrincipalTokensRepaid += amountDue;
        }

        //this will give collateral to the caller
        ITellerV2(TELLER_V2).lenderCloseLoanWithRecipient(_bidId, msg.sender);
    }

The amount being transfered is amountDue - tokensToGiveToSender

IERC20(principalToken).transferFrom(
                msg.sender,
                address(this),
                amountDue - tokensToGiveToSender
            );

but when thetotalPrincipalTokensRepaid is increased, it is increased with the whole amountDue:

totalPrincipalTokensRepaid += amountDue;

Impact

This will lead to wrong accounting and mess up the calculations based on this variable:

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

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L422-L472

Tool used

Manual Review

Recommendation

Implement the following change:

totalPrincipalTokensRepaid += amountDue - tokensToGiveToSender;

[nevillehuang]

Invalid, computations are correct, the full amountDue is added to totalPrincipalTokensRepaid since the whole loan is closed when liquidateDefaultedLoanWithIncentive is called and so there is no outstanding loans.