duc - Should use safeTransfer/safeTransferFrom instead of transfer/transferFrom

[github-actions[bot]]

duc

medium

Should use safeTransfer/safeTransferFrom instead of transfer/transferFrom

Summary

Calling transfer/transferFrom ERC20 without checking if it succeeds is very risky.

Vulnerability Detail

It is good to add a require() statement that checks the return value of token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.

Impact

In some cases, transfer/transferFrom without checking can lead to tokens being transferred fail but it will not revert. It is very dangerous to the protocol.

Code Snippet

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/core/UXDController.sol#L337

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#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#L512

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#L626

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

Tool used

Manual Review

Recommendation

Consider using safeTransfer/safeTransferFrom or require() consistently.