R2 - Inconsistent work with ``DnGmxSeniorVault`` in ``RageDnDepository``

[github-actions[bot]]

R2

high

Inconsistent work with DnGmxSeniorVault in RageDnDepository

Summary

Inconsistent work with assets/shares leads to users funds loss

Vulnerability Detail

DnGmxSeniorVault inherits from ERC4626Upgradeable And in assets/shares terminology users collateral token (e.g. USDC) is an asset (assertToken), which you deposits to the vault. And some DnGmxSeniorVault ERC-20 token is a shares token

So you deposit asserts and get shares Then you withdraw assers (by vault.withdraw()) and it burns your shares Shares represents which part of totals vault assets is yours

But you don't event save you shares returned in vault.deposit() function call. You user assets as shares and it may lead to big problems:

1. If rage-trade protocol will add fees (it's possible because they use proxy pattern) your protocol will be broken
2. If any other not-obvious changes happen to rage-trade protocol, you protocol behaviour will be unpredictable

Impact

Possibility of protocol DoS in case of adding fees or some unpredictable logic by rage-trade protocol

Code Snippet

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

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

Tool used

Manual Review

Recommendation

1. Save shares returned from vault.deposit()
2. In RageDnDepository. redeem() use vault.redeem() and user saved shares instead of using vault.withdraw()

[WarTech9]

RageTrade shares are held by the depository. It is not the intention to have the user redeem() the shares created when they deposited. Rather, the user redeems a fixed amount of asset to the amount of UXD they redeem (1:1). So our usecase users redeem for a given number of assets, not shares.

[hrishibhat]

Closing the issue based on Sponsor's comment.