HonorLt - Unsafe type casting

[github-actions[bot]]

HonorLt

medium

Unsafe type casting

Summary

The codebase contains several potential unsafe type casting that under certain conditions might brick the system.

Vulnerability Detail

There are some places in the code where it is assumed that the value will always fit into a narrow type so explicit casting is used, e.g.:

    function _processQuoteMint(uint256 quoteAmount) private returns (uint256) {
        uint256 normalizedAmount = quoteAmount.fromDecimalToDecimal(
            ERC20(quoteToken).decimals(),
            18
        );
        _checkNegativePnl(normalizedAmount);
        quoteMinted += int256(normalizedAmount);

    function getUnrealizedPnl() public view returns (int256) {
        return int256(redeemableUnderManagement) - int256(getPositionValue());
    }

    function _rebalanceNegativePnlWithSwap(
        uint256 amount,
        uint256 amountOutMinimum,
        uint160 sqrtPriceLimitX96,
        uint24 swapPoolFee,
        address account
    ) private returns (uint256, uint256) {
  ...
  int256 shortFall = int256(
            quoteAmount.fromDecimalToDecimal(18, ERC20(quoteToken).decimals())
        ) - int256(quoteAmountOut);
  ...

    function getUnrealizedPnl() public view returns (int256) {
        return int256(getDepositoryAssets()) - int256(netAssetDeposits);
    }

While realistically it is not likely to deal with very large values, with smart contracts it is never a good practice to assume something, better always check and take extra precautions.

Impact

If the actual value does not fit in the new type, it will be truncated and will lead to the messed up accounting of the protocol. The likelihood is very low but the impact would be critical thus I think this issue deserves to be of Medium severity.

Code Snippet

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

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

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

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

Tool used

Manual Review

Recommendation

Use the Safe casting library from OZ when changing types.

[IAm0x52]

Escalate for 25 USDC

Should be low or info. No legitimate risk of overflow for USDC, UXD or WETH in any possible use case.

int256.max is:

57896044618658097711785492504343953926634992332820282019728792003956564819967

or

57,896,044,618,658,097,711,785,492,504,343,953,926,634,992,332,820,282,019,728 full units of an 18 db token which is ~50**10 higher than the total supply of ETH

[sherlock-admin]

Escalate for 25 USDC

Should be low or info. No legitimate risk of overflow for USDC, UXD or WETH in any possible use case.

int256.max is:

57896044618658097711785492504343953926634992332820282019728792003956564819967

or

57,896,044,618,658,097,711,785,492,504,343,953,926,634,992,332,820,282,019,728 full units of an 18 db token which is ~50**10 higher than the total supply of ETH

You've created a valid escalation for 25 USDC!

To remove the escalation from consideration: Delete your comment. To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[hrishibhat]

Escalation accepted.

Agreed that this issue should be low as there is no risk given the use of tokens mentioned.

[sherlock-admin]

Escalation accepted.

Agreed that this issue should be low as there is no risk given the use of tokens mentioned.

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.