hansfriese - There is no option to manage the depository when the `unrealizedPnl` is negative in `RageDnDepository.sol`.

[github-actions[bot]]

hansfriese

medium

There is no option to manage the depository when the unrealizedPnl is negative in RageDnDepository.sol.

Summary

There is no option to manage the depository when the unrealizedPnl is negative in RageDnDepository.sol.

Vulnerability Detail

The RageDnDepository deposits the asset token to the DnGmxSeniorVault and withdraws back when users redeem their redeemable tokens.

File: 2023-01-uxd-hansfriese\contracts\integrations\rage-trade\RageDnDepository.sol
099:     function deposit(address asset, uint256 assetAmount)
100:         external
101:         onlyController
102:         returns (uint256)
103:     {
104:         if (asset != assetToken) {
105:             revert UnsupportedAsset(asset);
106:         }
107:         netAssetDeposits += assetAmount;
108:         IERC20(assetToken).approve(address(vault), assetAmount);
109:         uint256 shares = vault.deposit(assetAmount, address(this));
110:         uint256 redeemableAmount = _assetsToRedeemable(assetAmount);
111:         redeemableUnderManagement += redeemableAmount;
112:         _checkSoftCap();
113:         emit Deposited(msg.sender, assetAmount, redeemableAmount, shares);
114:         return redeemableAmount;
115:     }

And there is an admin function to withdraw profits from the vault.

File: 2023-01-uxd-hansfriese\contracts\integrations\rage-trade\RageDnDepository.sol
162:     function withdrawProfits(address receiver) external onlyOwner nonReentrant {
163:         int256 pnl = getUnrealizedPnl();
164:         if (pnl <= 0) {
165:             revert NoProfits(pnl);
166:         }
167:         uint256 profits = uint256(pnl);
168:         vault.withdraw(profits, receiver, address(this));
169:         realizedPnl += profits;
170:     }

But the unrealized pnl might be negative when something went wrong in the vault and the below scenario would be possible.

1. Users deposit 100 USDC to the vault and the depository has 100 shares of vault. (so 1 share = 1 USDC)
2. By some kinds of attacks or sudden changes, 1 share = 2 USDC for a while.
3. The deposit admin withdraws 100 USDC profit by burning 50 shares using withdrawProfits().
4. After that, 1 share = 1 USDC again and the depository has only 50 shares for 50 USDC.
5. So users can redeem 50 USDC only and it means the redeemable token isn't backed by asset like this comment.

Impact

There is no option to recover the depository if something went wrong with the vault.

As a result, the depository might remain as insolvant.

Code Snippet

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

Tool used

Manual Review

Recommendation

I think there should an option to deposit asset to the vault directly without increasing netAssetDeposits.

And the admin can deposit the asset when the unrealized pnl is negative to make the depository work properly.

    function depositProfits(address asset, uint256 assetAmount)
        external
        onlyOwner
        returns (uint256)
    {
        if (asset != assetToken) {
            revert UnsupportedAsset(asset);
        }

        int256 pnl = getUnrealizedPnl();
        require(pnl < 0, "pnl should be negative");

        // We can check the total deposited value 
        depositedPnl += amount;
        require(depositedPnl <= realizedPnl, "Can't deposit more than profit");

        IERC20(assetToken).approve(address(vault), assetAmount);
        uint256 shares = vault.deposit(assetAmount, address(this));
        return shares;
    }

[WarTech9]

This is by protocol design. Possible use-case for insurance fund.

[hrishibhat]

Closing this based on Sponsor comment