429.md

hansfriese

medium

rebalanceLite should provide a slippage protection

Summary

Users can lose funds while rebalancing.

Vulnerability Detail

The protocol provides two kinds of rebalancing functions - rebalance() and rebalanceLite(). While the function rebalance() is protected from an unintended slippage because the caller can specify amountOutMinimum, rebalanceLite() does not have this protection. This makes the user vulnerable to unintended slippage due to various scenarios.

PerpDepository.sol
597:     function rebalanceLite(
598:         uint256 amount,
599:         int8 polarity,
600:         uint160 sqrtPriceLimitX96,
601:         address account
602:     ) external nonReentrant returns (uint256, uint256) {
603:         if (polarity == -1) {
604:             return
605:                 _rebalanceNegativePnlLite(amount, sqrtPriceLimitX96, account);
606:         } else if (polarity == 1) {
607:             // disable rebalancing positive PnL
608:             revert PositivePnlRebalanceDisabled(msg.sender);
609:             // return _rebalancePositivePnlLite(amount, sqrtPriceLimitX96, account);
610:         } else {
611:             revert InvalidRebalance(polarity);
612:         }
613:     }
614:
615:     function _rebalanceNegativePnlLite(
616:         uint256 amount,
617:         uint160 sqrtPriceLimitX96,
618:         address account
619:     ) private returns (uint256, uint256) {
620:         uint256 normalizedAmount = amount.fromDecimalToDecimal(
621:             ERC20(quoteToken).decimals(),
622:             18
623:         );
624:
625:         _checkNegativePnl(normalizedAmount);
626:         IERC20(quoteToken).transferFrom(account, address(this), amount);
627:         IERC20(quoteToken).approve(address(vault), amount);
628:         vault.deposit(quoteToken, amount);
629:
630:         bool isShort = false;
631:         bool amountIsInput = true;
632:         (uint256 baseAmount, uint256 quoteAmount) = _placePerpOrder(
633:             normalizedAmount,
634:             isShort,
635:             amountIsInput,
636:             sqrtPriceLimitX96
637:         );
638:         vault.withdraw(assetToken, baseAmount);
639:         IERC20(assetToken).transfer(account, baseAmount);
640:
641:         emit Rebalanced(baseAmount, quoteAmount, 0);
642:
643:         return (baseAmount, quoteAmount);
644:     }

Especially, according to the communication with the PERP dev team, it is possible for the Perp's ClearingHouse to fill the position partially when the price limit is specified (sqrtPriceLimitX96). It is also commented in the Perp contract comments here.

63:     /// @param sqrtPriceLimitX96 tx will fill until it reaches this price but WON'T REVERT
64:     struct InternalOpenPositionParams {
65:         address trader;
66:         address baseToken;
67:         bool isBaseToQuote;
68:         bool isExactInput;
69:         bool isClose;
70:         uint256 amount;
71:         uint160 sqrtPriceLimitX96;
72:     }

So it is possible that the order is not placed to the full amount. As we can see in the #L626~#L628, the UXD protocol grabs the quote token of amount and deposits to the Perp's vault. And the unused amount will remain in the Perp vault while this is supposed to be returned to the user who called this rebalance function.

Impact

Users can lose funds while lite rebalancing.

Code Snippet

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

Tool used

Manual Review

Recommendation

Add a protection parameter to the function rebalanceLite() so that the user can specify the minimum out amount.