sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

TomJ - Unhandled Return Values of transfer and transferFrom #430

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

TomJ

medium

Unhandled Return Values of transfer and transferFrom

Summary

Unsafe transfer and transferFrom functions are used in the contract.

Vulnerability Detail

There are functions in which ERC20 token transfer and tranferFrom is not checking the return values. Since some of the ERC20 tokens does not revert on failure but return false instead, it is important to check the return values or otherwise these tokens is still counted as a correct transfer even thought it didn't actually perform the transfer.

Impact

Function execution will not revert for certain tokens even though the ERC20 transfer failed which will cause loss of funds for protocol and user.

Code Snippet

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/core/UXDController.sol#L195-L199 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-L516 https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L519-L522 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

I recommend to add a require() statement that checks the return values or use OpenZeppelin's SafeERC20 wrapper functions.