sherlock-audit / 2024-02-leverage-contracts-judging

1 stars 0 forks source link

kgothatso - check usdt approve to zero #31

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

kgothatso

high

check usdt approve to zero

Summary

There are 3 instances where the IERC20.approve() function is called only once without setting the allowance to zero. Some tokens, like USDT, require first reducing the address' allowance to zero by calling approve(_spender, 0).

Vulnerability Detail

Transactions will revert

Impact

Transactions will revert when using an unsupported token like USDT. Some token contracts do not return any value. Some token contracts revert the transaction when the allowance is not zero.

Code Snippet

https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/abstract/ApproveSwapAndPay.sol#L66

USDT approve method (it checks that allowance is zero):

function approve( address _spender, uint _value ) public onlyPayloadSize(2 * 32) { ... require(!((_value != 0) && (allowed[msg.sender][_spender] != 0))); allowed[msg.sender][_spender] = _value; ... }

Tool used

Manual Review

Recommendation

I recommend doing approve(0) before calling the main approve (currently OpenZeppelin contracts have this logic https://github.com/OpenZeppelin/openzeppelin-contracts/blob/a714fe6dbd84d1f8016cce922fb719494afa9ba2/contracts/token/ERC20/utils/SafeERC20.sol#L52).

Recommend to use safeApprove instead and set the allowance to 0 before calling it.

function approveUnderlying(address spender) private {
    for (uint256 i = 0; i < weights.length; i++) {
        IERC20(tokens[i]).safeApprove(spender, 0);
        IERC20(tokens[i]).safeApprove(spender, type(uint256).max);
    }
}

Duplicate of #9

sherlock-admin2 commented 6 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: medium(2)