Closed sherlock-admin3 closed 2 months ago
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
FastTiger
medium
Failure to Reset Allowance to Zero Before Changing it
Summary
The allowance was not reset to zero before modifying it, which could lead to issues with certain ERC20 tokens like USDT.
Vulnerability Detail
Certain ERC20 tokens, such as USDT, do not allow changing the allowance from a non-zero value without first setting it to zero. For instance, Tether (USDT)'s approve() function will revert if the current approval is not zero to prevent front-running changes in approvals.
Impact
Several functionalities within the vaults may not work as expected 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 advised to always reset the allowance to zero before modifying it and consider using
safeApprove
orsafeIncreaseAllowance
to handle allowance changes safely.