When redeem shares, maxRedeem of user is not checked

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xf7a015f3ddf4340c4b05c50a50ed383c0990d8ac664a64fdd0e0f799156cf447 Severity: low

Description: Description\

redeemAtom() is used to redeem shares from an atom vault and redeemTriple() is used to redeem shares number of shares from the triple vault.

There function implementation can be checked here and here. If noticed, both of these function does not check the maximum redeem of shares of function caller i.e msg.sender.

maxRedeem() returns maximum amount of shares that can be redeemed from the 'owner' balance or function caller i.e msg.sender balance through a redeem call.

    function maxRedeem(address owner, uint256 id) external view returns (uint256) {
        uint256 shares = vaults[id].balanceOf[owner];
        return shares;
    }

This max redeem is not checked in both redeemAtom() and redeemTriple() functions.

As per EIP-4626, see redeem specificication here

MUST revert if all of shares cannot be redeemed (due to withdrawal limit being reached, slippage, the owner not having enough shares, etc).

so this requirement of EIP4626 must be taken into consideration and function should revert for above highlighted case.

It should be noted that, EthMultiVault is conformed to be EIP4626 vaults based on below documentation references:

Conforming to the ERC4626 Tokenized Vault Standard, we extend the functionality to our MultiVault standard.

Intuition extends the ERC-4626 standard with the EthMultiVault.Sol contract

Check this reference.

Recommendation to fix\ Consider below changes in both redeemAtom() and redeemTriple() functions.

In redeemAtom():

    function redeemAtom(uint256 shares, address receiver, uint256 id) external nonReentrant returns (uint256) {
        if (id == 0 || id > count) {
            revert Errors.MultiVault_VaultDoesNotExist();
        }
+      require(shares < maxRedeem(msg.sender,id), "insufficient shares");

       . . . some code . . . 

}

and in redeemTriple():

    function redeemTriple(uint256 shares, address receiver, uint256 id) external nonReentrant returns (uint256) {
        if (!isTripleId(id)) {
            revert Errors.MultiVault_VaultNotTriple();
        }
+      require(shares < maxRedeem(msg.sender,id), "insufficient shares");

       . . . some code . . . 

}

[mihailo-maksa]

The reported issue highlights that the redeemAtom and redeemTriple functions do not explicitly check the maximum redeemable shares of the user, potentially allowing over-redemption.

Label: minor

Comment: This check is indeed implemented in the _redeem function, which both redeemAtom and redeemTriple call. The _redeem function checks the user’s share balance and reverts if there are insufficient shares:

if (vaults[id].balanceOf[owner] < shares) {
    revert Errors.MultiVault_InsufficientSharesInVault();
}

The suggested enhancement is to explicitly use maxRedeem for clarity and compliance with ERC-4626, although our current implementation already ensures the intended outcome. A small payout might be considered as the suggested fix aligns with the ERC-4626 standard.

Comment on the issue: This check is indeed implemented in the _redeem function, ensuring users cannot redeem more shares than they hold. We might consider a small payout since the suggested fix aligns with ERC-4626 compatibility, even though our implementation already achieves the desired outcome.