Deivitto - ERC20 `approve` fail for some tokens

[github-actions[bot]]

Deivitto

medium

ERC20 approve fail for some tokens

Summary

Some tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.

Also approve() will fail for certain token implementations that do not return a boolean value. Hence it is recommend to use increaseAllowance() and decreaseAllowance()

Vulnerability Detail

approve reverts for tokens like USDT if first not approved to 0 Use of approve over the code incosistently and not considered as best practice

Impact

Reverting in some tokens like USDT, approve returning value not noticed

Code Snippet

• Some ERC20 like USDT fail if not set first to 0

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/governance/UXDGovernor.sol#L198

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/governance/UXDTimelockController.sol#L52

• In this ones happens the same but also approve returning value is ignored

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L198

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L286

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L394

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L627

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/rage-trade/RageDnDepository.sol#L108

Tool used

Manual Review

Recommendation

• Add 0 approve before approve
• Consider using increaseAllowance and decreaseAllowance instead of approve
• Check return value values and revert/emit events if needed