Exit Fee Not Deducted from Total Assets in Redeem Function

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0xfe48c86ee5fde1eaae0770dab56021baee6c579c2aa94c61a309134d82982c86
Severity: high

Description:
Description
In the _redeem function of the EthMultiVault contract, there's a critical issue where the exit fee is not deducted from the total assets when updating the vault totals. This discrepancy can lead to an accounting mismatch between the actual assets in the vault and the recorded total assets, potentially causing fund imbalances and incorrect share price calculations.

Attack Scenario\

1. A user calls the redeemAtom or redeemTriple function to withdraw their shares.
2. The _redeem function calculates the assets to be returned to the user, including the exit fee and protocol fee.
3. When updating the vault totals, the function deducts the assetsForReceiver and protocolFee from the total assets, but fails to deduct the exitFee.
4. Over time, as more users redeem their shares, the discrepancy between the recorded total assets and the actual assets in the vault grows.
5. This can lead to:
   • Inflated share prices for remaining users
   • Potential insolvency of the vault if all users try to redeem
   • Incorrect calculations in other functions that rely on the total assets value

Attachments

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "./EthMultiVault.sol";

contract EthMultiVaultExploit {
    EthMultiVault public vault;

    constructor(address _vaultAddress) {
        vault = EthMultiVault(_vaultAddress);
    }

    function exploitRedeem(uint256 vaultId, uint256 shares) external {
        // Assume we have shares in the vault
        uint256 initialTotalAssets = vault.vaults(vaultId).totalAssets;
        
        vault.redeemAtom(shares, address(this), vaultId);
        
        uint256 finalTotalAssets = vault.vaults(vaultId).totalAssets;
        
        // The difference between initialTotalAssets and finalTotalAssets
        // will be less than it should be, as the exit fee is not deducted
        
        // Over time, this discrepancy will grow, leading to an inflated
        // totalAssets value in the vault
    }
}

2. Revised Code File (Optional)

function _redeem(uint256 id, address owner, uint256 shares) internal returns (uint256, uint256) {
    // ... (previous code remains the same)

    (, uint256 assetsForReceiver, uint256 protocolFee, uint256 exitFee) = getRedeemAssetsAndFees(shares, id);

    // Corrected: Include exitFee in the totalAssetsDelta calculation
    _setVaultTotals(
        id,
        vaults[id].totalAssets - (assetsForReceiver + protocolFee + exitFee), // totalAssetsDelta
        vaults[id].totalShares - shares // totalSharesDelta
    );

    // ... (rest of the function remains the same)
}

[mihailo-maksa]

The issue suggests that the exit fee is not deducted from the total assets when updating vault totals in the _redeem function.

Label: invalid

Comment:
The exitFee is designed to stay as part of the totalAssets of the specific vault to reward the remaining shareholders with a higher share price. This is an intentional design choice and does not pose a security risk.

Comment on the issue: The exitFee is intentionally kept as part of the totalAssets to reward remaining shareholders with a higher share price. This is by design and not a security vulnerability.