DenTonylifer - Protocol may not work with USDT

[sherlock-admin4]

DenTonylifer

medium

Protocol may not work with USDT

Summary

Protocol may not work with USDT.

Vulnerability Detail

Taking into account the information from the README and the comments of the sponsor, it can be concluded that the protocol will support such tokens as USDT.
sherlock-audit/2023-03-teller-judging#423 (comment)
Wardens have previously highlighted problems with USDT due to usage transfer() instead of safeTransfer():
sherlock-audit/2023-03-teller-judging#220
The sponsor has fixed this issue, so users can now create successful loans using USDT as collateral. But some functions from the new contracts still use the deprecated token transfer method, which can cause some problems.
For example, the lender will not be able to liquidate defaulted loan using LenderCommitmentGroup_Smart.liquidateDefaultedLoanWithIncentive() function, if principal token is USDT:

function liquidateDefaultedLoanWithIncentive(
        uint256 _bidId,
        int256 _tokenAmountDifference
    ) public bidIsActiveForGroup(_bidId) {
        // . . . 

        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 {
           // . . .
        }

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

Also borrower will not be able to rollover their existing loan using a flash loan mechanism in FlashRolloverLoan_G5.sol:

function rolloverLoanWithFlash(
        address _lenderCommitmentForwarder,
        uint256 _loanId,
        uint256 _flashLoanAmount,    // - USDT amount
        uint256 _borrowerAmount, //an additional amount borrower may have to add    // - USDT amount
        AcceptCommitmentArgs calldata _acceptCommitmentArgs
    ) external   {
        address borrower = TELLER_V2.getLoanBorrower(_loanId);
        require(borrower == msg.sender, "CommitmentRolloverLoan: not borrower");

        // Get lending token and balance before
        address lendingToken = TELLER_V2.getLoanLendingToken(_loanId);    // - USDT

        if (_borrowerAmount > 0) {
            IERC20(lendingToken).transferFrom(   <<<--------------------
                borrower,
                address(this),
                _borrowerAmount    // - USDT amount
            );
        }

        // Call 'Flash' on the vault to borrow funds and call tellerV2FlashCallback
        // This ultimately calls executeOperation
        IPool(POOL()).flashLoanSimple(
            address(this),
            lendingToken,    // - USDT
            _flashLoanAmount,   // - USDT amount
            abi.encode(
                RolloverCallbackArgs({
                    lenderCommitmentForwarder :_lenderCommitmentForwarder,
                    loanId: _loanId,
                    borrower: borrower,
                    borrowerAmount: _borrowerAmount,
                    acceptCommitmentArgs: abi.encode(_acceptCommitmentArgs)
                })
            ),
            0 //referral code
        );
    }

Impact

The functions mentioned above will fall, because USDTreturns void instead of bool, thus transferFrom() function of the IERC20 interface will revert.

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/FlashRolloverLoan_G5.sol#L111
https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/FlashRolloverLoan_G5.sol#L205
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#L441-L472

Tool used

Manual Review

Recommendation

Replace the the transferFrom() function with the safeTransferFrom() function.

Duplicate of #50