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.
https://github.com/sherlock-audit/2023-03-teller-judging/issues/423#issuecomment-1535401214
Wardens have previously highlighted problems with USDT due to usage transfer() instead of safeTransfer():
https://github.com/sherlock-audit/2023-03-teller-judging/issues/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.
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
. https://github.com/sherlock-audit/2023-03-teller-judging/issues/423#issuecomment-1535401214 Wardens have previously highlighted problems withUSDT
due to usagetransfer()
instead ofsafeTransfer()
: https://github.com/sherlock-audit/2023-03-teller-judging/issues/220 The sponsor has fixed this issue, so users can now create successful loans usingUSDT
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 usingLenderCommitmentGroup_Smart.liquidateDefaultedLoanWithIncentive()
function, if principal token isUSDT
:Also borrower will not be able to rollover their existing loan using a flash loan mechanism in
FlashRolloverLoan_G5.sol
:Impact
The functions mentioned above will fall, because
USDT
returnsvoid
instead ofbool
, thustransferFrom()
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 thesafeTransferFrom()
function.Duplicate of #50