UXD

UXDController::redeem should allow to redeem any amount of UXD tokens (up to the user’s balance). It works by calling PerpDepository::openLong which calls clearingHouse::openPosition to open a long position, then it burns the amount of quoteToken returned by clearingHouse::openPosition and transfers to the user the corresponding amount of base token returned by openPosition.

Note that clearingHouse::openPosition, when called with isBaseToQuote = false, isExactInput = true, is expected to return a quoteAmount equal to the input amount. However, it seems that for certain input amounts, the returned quoteAmount is 1 wei larger than the input amount.

This discrepancy has the consequence that UXDController::redeem tries to burn 1 more wei than the requested amount. This likely will fail, because the user approves the exact amount he wishes to redeem (not 1 wei larger). This issue is reproducible, for instance, when trying to redeem 1280383806188353801279 tokens.

function _redeem(InternalRedeemParams memory redeemParams)
    internal
    returns (uint256, uint256)
{
    ...
    (uint256 base, uint256 quote) = IDepository(depository).openLong(
   	 positionParams
    );
    // Dedaub: Τhis will fail when quote > redeemParams.amountToRedeem
    if (redeemable.allowance(redeemParams.account, address(this)) < quote)
    {
   	 revert CtrlNotApproved(address(redeemable),
                              redeemParams.account,
                              quote);
    }
}

Although not a security vulnerability, this is an important issue that prevents users from accessing a core functionality of the protocol. It might be particularly problematic if the protocol is used by other smart contracts, instead of EOAs, which might not have the possibility to workaround this issue by adjusting the input amount.

When rebalancing via a swap, it is assumed that the spot market will return an amount of tokens smaller or equal to the one needed. The caller will then need to provide the difference.

function _rebalancePositivePnlWithSwap(
    uint256 amount,
    uint256 amountOutMinimum,
    uint160 sqrtPriceLimitX96,
    uint24 swapPoolFee,
    address account
) internal returns (uint256, uint256) {
    ...
    // Dedaub: The following will fail if baseAmount < baseAmountFromSwap
    uint256 shortFall = baseAmount - baseAmountFromSwap;
    if (shortFall > 0) {
   	 IERC20(baseToken).transferFrom(account, address(this), shortFall);
   	 vault.deposit(baseToken, shortFall);
    }
}

Although this behaviour is indeed expected due to swapping fees, the Perp and swap markets are two independent markets and it could be the case that the prices are not always fully in sync with each other. If the spot market returns even 1 wei more than expected, it will be impossible to execute a rebalance due to arithmetic underflow.

We believe that it would be safe to treat such edge cases in the code.

PerpDepository::_placePerpOrder seems to set a deadline for the Perp order to be executed within the next DEFAULT_DEADLINE seconds.

function _placePerpOrder(
    uint256 amount,
    bool isShort,
    bool amountIsInput,
    uint160 sqrtPriceLimit
) internal returns (uint256, uint256) {
    uint256 upperBound = 0; // 0 = no limit, limit set by sqrtPriceLimit

    IClearingHouse.OpenPositionParams memory params = IClearingHouse
   	 .OpenPositionParams({
             // Dedaub: This has no effect
   		 deadline: block.timestamp + DEFAULT_DEADLINE,
             ...
   	 });
}

We are not sure what is the purpose of this deadline, but, the way it is used, this argument has practically no effect and could be misleading. Setting a fixed deadline could be useful in a transaction submitted by an EOA, to restrict the time of execution. But when executed by a smart contract, block.timestamp will always be the time of the current block, no matter which block the transaction will appear in. A user, submitting, for instance, a mint request, might have the expectation that it has to be executed within a certain time, but in reality no such restriction is enforced.

If setting a deadline is not needed, we recommend passing deadline = block.timestamp to make the code clearer and remove any false expectations.

If, on the other hand, setting a deadline is needed, it should be received as an argument to the corresponding external methods and forwarded to PerpDepository::_placePerpOrder.

The idea of rebalancing positive PnL in the documentation is to withdraw the USDC profits from Perp, convert them to ETH, and use them to increase the size of the position. However, the way it is implemented, rebalancing cannot directly use the profits but requires USDC deposits to be already present in the protocol’s Perp vault.

Consider for instance PerpDepository::_rebalancePositivePnlWithSwap

function _rebalancePositivePnlWithSwap(
    uint256 amount,
    uint256 amountOutMinimum,
    uint160 sqrtPriceLimitX96,
    uint24 swapPoolFee,
    address account
) internal returns (uint256, uint256) {

    uint256 normalizedAmount = amount.fromDecimalToDecimal(
   	 ERC20(quoteToken).decimals(),
   	 18
    );
    _checkPositivePnl(normalizedAmount);

    // Dedaub: This withdrawal cannot use the profits,
    //         USDC need to be already available in the vault
    vault.withdraw(quoteToken, amount);
    ...
}

Although the position has unrealized profits, these profits cannot be withdrawn with vault.withdraw until the position is (at least partially) closed. So the withdrawal effectively withdraws USDC collateral from the insurance deposits, if no such collateral exists rebalancing will fail.

The same is true for PerpDepository::_rebalancePositivePnlLite.

function _rebalancePositivePnlLite(
    uint256 amount,
    uint160 sqrtPriceLimitX96,
    address account
) internal returns (uint256, uint256) {
    uint256 normalizedAmount = amount.fromDecimalToDecimal(
   	 ERC20(quoteToken).decimals(),
   	 18
    );
    _checkPositivePnl(normalizedAmount);
    bool isShort = true;
    bool amountIsInput = false;
    (uint256 baseAmount, uint256 quoteAmount) = _placePerpOrder(
   	 normalizedAmount,
   	 isShort,
   	 amountIsInput,
   	 sqrtPriceLimitX96
    );
    IERC20(baseToken).transferFrom(account, address(this), baseAmount);
    IERC20(baseToken).approve(address(vault), baseAmount);
    vault.deposit(baseToken, baseAmount);

    // Dedaub: This will fail unless the vault
    //         already contains quoteToken
    vault.withdraw(quoteToken, amount);
}

Since the position is only increased, the profits are not released and the withdrawal will fail unless USDC collateral already exists in the vault.

This behaviour might be problematic if the insurance funds are removed or are not sufficient. Moreover, it has the side-effect that, after a rebalance, part of the insurance funds will be in the form of Perp profits, so it will be impossible to fully withdraw them until the Perp position is partially closed.

An alternative approach could be to first partially close the position, release the USDC profits, perform the rebalancing and open it again.

Using the rebalancing operations (which can be externally called), an adversary can force the contract to perform swaps with slippage parameters controlled by him. Contracts that allow such forced swaps are often vulnerable to sandwich-like attacks: an adversary could first tilt a pool, e.g. sell ETH to make it very cheap, force the protocol to sell its ETH, then buy the ETH back for a profit.

In the case of UXD, although forced swaps are possible, they don’t seem to be exploitable for the following reasons:

• In the case of rebalancing with swap, the caller needs to cover any shortfall
• In the case of lite rebalancing, the caller himself acts as an exchange.

Still, the ability to force swaps is not a safe practice and could potentially lead to vulnerabilities in the future. It might be worth implementing counter-measures, such as limiting the amount that can be rebalanced, or even marking these operations as onlyOwner.

The code can be compiled with Solidity 0.8.9 or higher. For deployment, we recommend no floating pragmas, but a specific version, to be confident about the baseline guarantees offered by the compiler. Version 0.8.9, in particular, has some known bugs, which we do not believe affect the correctness of the contracts.