sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

Deivitto - erc20 not checked on transfer #445

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Deivitto

medium

erc20 not checked on transfer

Summary

erc20 not checked on transfer

Vulnerability Detail

ERC20 implementations are not always consistent. Some implementations of transfer and transferFrom could return false on failure instead of reverting. It is safer to wrap such calls into require() statements or use safe wrapper functions implementing return value/data checks to handle these failures. For reference, see similar Medium-severity finding from Consensys Diligence Audit of Aave Protocol V2.

While the contract uses Uniswap's TransferHelper library function safeTransfer in other places for ERC20 tokens (which call the token's transfer / transferFrom functions and check return value for success and return data), it misses using TransferHelper.safeTransferFrom in some cases without checking for its return value.

See similar issue previously reported

Impact

Lose of assets

Code Snippet

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

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

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

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

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/core/UXDController.sol#L195

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

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

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

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/core/UXDController.sol#L225

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

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

Tool used

Manual Review

Recommendation

- IERC20(token).transferFrom(msg.sender, address(this), amount);
+ TransferHelper.safeTransferFrom(token, msg.sender, address(this), amount);

As already done in Uniswapper