code-423n4 / 2022-12-tigris-findings

8 stars 4 forks source link

ERC20 approvals may need to be set to 0 beforehand #632

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L652 https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Lock.sol#L117

Vulnerability details

Impact

There are some instances where there is an ERC20 approval for a max uint256 amount. ERC20 tokens such as USDT require the address allowance to be set to 0 beforehand, so this would cause reverts for those tokens.

Proof of Concept

-Token such as USDT gets whitelisted -Normal call for to stableVault contract will revert, via https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L652 since USDT requires an approval of 0 first
-This can also apply in the Lock, asset distribution to bondNFT contract: https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Lock.sol#L117

Tools Used

None

Recommended Mitigation Steps

In the above scenarios, precede the corresponding lines with the line: IERC20(_asset).approve(_spender) ,0) to account for that edge case.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #104

c4-judge commented 1 year ago

GalloDaSballo marked the issue as satisfactory