sherlock-audit / 2023-11-convergence-judging

8 stars 8 forks source link

chainNue - Allowance is not set to zero first before approving #203

Closed sherlock-admin closed 10 months ago

sherlock-admin commented 10 months ago

chainNue

medium

Allowance is not set to zero first before approving

Summary

approveTokens did not Approve to zero first. Some ERC20 tokens like USDT require resetting the approval to 0 first before being able to reset it to another value.

Vulnerability Detail

Some non-standard ERC20 tokens, such as USDT, require approve 0 first, otherwise they will revert. The approveTokens method in SdtUtilities is not handling this case which makes the modules incompatible with these tokens.

File: SdtUtilities.sol
214:     function approveTokens(TokenSpender[] calldata _tokenSpenders) external onlyOwner {
215:         for (uint256 i; i < _tokenSpenders.length; ) {
216:             IERC20(_tokenSpenders[i].token).approve(_tokenSpenders[i].spender, _tokenSpenders[i].amount);
217:             unchecked {
218:                 ++i;
219:             }
220:         }
221:     }

On Readme, it states, it will support USDT, thus this issue should be a valid medium one.

Impact

SdtUtilities is not compatible with certain ERC20 tokens for example USDT

Code Snippet

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/utils/SdtUtilities.sol#L214-L221

Tool used

Manual Review

Recommendation

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

Duplicate of #35