paspe - Token should be approved with zero amount first and then uint.max

[sherlock-admin]

paspe

medium

Token should be approved with zero amount first and then uint.max

Summary

The process of "approving zero" for an ERC-20 token is typically done when a user wants to change the amount of the token they have approved for a particular smart contract.

Vulnerability Detail

Some ERC-20 tokens, such as USDT and BUSD, require that you first set the allowance to zero before setting a new allowance value. This is known as the "double approval" process.

Impact

The reason for this is that some ERC-20 tokens, including USDT and BUSD, have a feature called "upgradable smart contracts". This means that the token contract can be upgraded to a new version, which can potentially change the way the contract interacts with the allowances that users have set.

Code Snippet

https://github.com/sherlock-audit/2023-05-dodo/blob/930565dd875dac24609441423c7c76c2ae4719a8/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L394

function _approveToken(address _address, address _tokenAddress, uint256 _tokenAmount) internal {
        if (IERC20(_tokenAddress).allowance(address(this), _address) < _tokenAmount) {
            IERC20(_tokenAddress).approve(_address, type(uint256).max);
        }
    }

Tool used

Manual Review

Recommendation

You can easily refactor it

function _approveToken(address _address, address _tokenAddress, uint256 _tokenAmount) internal {
        if (IERC20(_tokenAddress).allowance(address(this), _address) < _tokenAmount) {
            IERC20(_tokenAddress).approve(_address, 0);
            IERC20(_tokenAddress).approve(_address, type(uint256).max);
        }
    }