ApprovePayAndSwap::_maxApproveIfNecessary does not work with some ERC20 tokens
Summary
The ApprovePayAndSwap::_maxApproveIfNecessary relies on the boolean returned by the ERC20 approve function. However, some tokens do not return a boolean, and instead revert the transaction. If such an ERC20 token is used, then the following code in the ApprovePayAndSwap::_maxApproveIfNecessary will not execute, therefore not allowing allowances to be set on these tokens.
Vulnerability Detail
The ApprovePayAndSwap::_maxApproveIfNecessary function ensures that the allowance for a spender is at least the specified amount. If the current allowance is less than the specified amount, it attempts to approve the maximum possible value, and if that fails, it retries with the maximum possible value minus one. If both attempts fail, it reverts with an error indicating that the approval did not succeed.
It uses many if statements to try and approve allowances, relying on the boolean returned by the ERC20 approve function, which is called in _tryApprove. If a token is used that does not return a boolean and instead reverts, the following if statements will not be executed, not allowing allowances to be set.
Impact
Denial of service of the _maxApproveIfNecessary function (since the following if statements cannot execute) for these tokens and inability to set allowances on them.
As you can see, the if statements in _maxApproveIfNecessary check the bool returned by _tryApprove and depending on that value, it executes the next if statement. However, if such an ERC20 token does not return a bool on approve, then the function will revert.
crypticdefense
medium
ApprovePayAndSwap::_maxApproveIfNecessarydoes not work with some ERC20 tokensSummary
The
ApprovePayAndSwap::_maxApproveIfNecessaryrelies on the boolean returned by the ERC20 approve function. However, some tokens do not return a boolean, and instead revert the transaction. If such an ERC20 token is used, then the following code in theApprovePayAndSwap::_maxApproveIfNecessarywill not execute, therefore not allowing allowances to be set on these tokens.Vulnerability Detail
The
ApprovePayAndSwap::_maxApproveIfNecessaryfunction ensures that the allowance for a spender is at least the specified amount. If the current allowance is less than the specified amount, it attempts to approve the maximum possible value, and if that fails, it retries with the maximum possible value minus one. If both attempts fail, it reverts with an error indicating that the approval did not succeed.It uses many
ifstatements to try and approve allowances, relying on the boolean returned by the ERC20 approve function, which is called in_tryApprove. If a token is used that does not return a boolean and instead reverts, the followingifstatements will not be executed, not allowing allowances to be set.Impact
Denial of service of the
_maxApproveIfNecessaryfunction (since the followingifstatements cannot execute) for these tokens and inability to set allowances on them.Code Snippet
https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/abstract/ApproveSwapAndPay.sol#L82-L95
https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/abstract/ApproveSwapAndPay.sol#L66-L71
As you can see, the
ifstatements in_maxApproveIfNecessarycheck the bool returned by_tryApproveand depending on that value, it executes the nextifstatement. However, if such an ERC20 token does not return a bool on approve, then the function will revert.Tool used
Manual Review
Recommendation
Use Openzeppelin SafeTransfer / SafeApprove
Duplicate of #5