blockchain555 - Did Not Approve To Zero First

[sherlock-admin3]

blockchain555

medium

Did Not Approve To Zero First

Summary

Allowance was not set to zero first before changing the allowance.

Vulnerability Detail

Some ERC20 tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value. For example Tether (USDT)'s approve() function will revert if the current approval is not zero, to protect against front-running changes of approvals.

The following attempt to call the approve() function without setting the allowance to zero first.

    function acceptFundsForAcceptBid(
        address _borrower,
        uint256 _bidId,
        uint256 _principalAmount,
        uint256 _collateralAmount,
        address _collateralTokenAddress,
        uint256 _collateralTokenId, 
        uint32 _loanDuration,
        uint16 _interestRate
    ) external onlySmartCommitmentForwarder whenNotPaused {
        
        SNIP...
 
373:    principalToken.approve(address(TELLER_V2), _principalAmount);

        //do not have to spoof/forward as this contract is the lender !
        _acceptBidWithRepaymentListener(_bidId);

        SNIP...
    }

Impact

A number of features within the vaults will not work if the approve function reverts.

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#L336-L382

Tool used

Manual Review

Recommendation

It is recommended to set the allowance to zero before increasing the allowance and use safeApprove/safeIncreaseAllowance.

[nevillehuang]

After further review, since acceptFundsForAcceptBid can only be invoked by the forwarder, If a lender calls acceptCommitmentWithRecipient via the forwarder, it will definitely consume all approvals as seen here. So this issue will never occur