sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

ak1 - PerpDepository.sol : transfer's return value is not validated #391

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

ak1

medium

PerpDepository.sol : transfer's return value is not validated

Summary

In following places transfer function call is used to transfer the asset in PerpDepository.sol .

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

The issue here is, for some tokens if added as asset, the transfer status will be decided based on the return statement from the transfer call.

I believe that the UXD would accepts wide range of assets as time goes on due to increasing the number of valid tokens. Though, the current implementation is interacting only with WETH and USDC, I would like to flag this as issue by considering the future scope of the project.

For some reference, https://github.com/d-xo/weird-erc20#:~:text=Missing%20Return%20Values This has basic examples about how this would come possible.

From IERC20.sol,

/**
 * @dev Moves `amount` tokens from the caller's account to `to`.
 *
 * Returns a boolean value indicating whether the operation succeeded.
 *
 * Emits a {Transfer} event.
 */
function transfer(address to, uint256 amount) external returns (bool);

Vulnerability Detail

Refer the summary section.

Impact

For some asset tokens, even if the transfer failed, it will be treated as succeeded.

Code Snippet

Refer the summary section.

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

Tool used

Manual Review

Recommendation

I saw the protocol is handling it correctly in UXDTimelockController.sol .

but not in the PerpDepository.sol.