sherlock-audit / 2023-03-taurus-judging

4 stars 0 forks source link

w42d3n - The function approveTokens() do not approve To Zero first #183

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

w42d3n

medium

The function approveTokens() do not approve To Zero first

Summary

The code approves non-zero and non-uint256.max amount for a token without first approving 0. This will always revert if the token is USDT.

Vulnerability Detail

Some ERC20 tokens 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.

More info: https://github.com/d-xo/weird-erc20#approval-race-protections

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

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/LiquidationBot/LiquidationBot.sol#LL59-L61

    function approveTokens(address _tokenIn, address _vault) external onlyMultisig {
        IERC20(_tokenIn).approve(address(_vault), type(uint256).max);
    }

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/SwapAdapters/UniswapSwapAdapter.sol#L53-L55

    function approveTokens(address _tokenIn) external {
        IERC20(_tokenIn).approve(swapRouter, type(uint256).max);
    }

However, if the token involved is an ERC20 token that does not work when changing the allowance from an existing non-zero allowance value, it will break a number of key functions or features of the protocol as the approveTokens function is critical for the protocol.

Impact

The function approvetokens() will always revert if the token is USDT

Code Snippet

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/LiquidationBot/LiquidationBot.sol#LL59-L61

    function approveTokens(address _tokenIn, address _vault) external onlyMultisig {
        IERC20(_tokenIn).approve(address(_vault), type(uint256).max);
    }

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/SwapAdapters/UniswapSwapAdapter.sol#L53-L55

    function approveTokens(address _tokenIn) external {
        IERC20(_tokenIn).approve(swapRouter, type(uint256).max);
    }

Tool used

Manual Review

Recommendation

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

ex:

    function approveTokens(address _tokenIn, address _vault) external onlyMultisig {
        ++ IERC20(_tokenIn).approve(address(_vault), 0);
        IERC20(_tokenIn).approve(address(_vault), type(uint256).max);
    }

Duplicate of #134