the `minDeposit` can be bypassed in `ETHmultivault.sol`

[hats-bug-reporter[bot]]

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

Description: Description\

minDeposit is created to prevent users from depositing under a specific amount which set from protocol side trough functions

but the problem is the logic of minDeposit is easily bypasable because of the wrong condition check

here in depositAtom function the condition check of if (msg.value < generalConfig.minDeposit) done before reducing the fee amount thus it acually makes it possible to bypass the minDeposit logic by just setting minimum value to just same as minDeposit and then the protocol fee will be subtracted from the msg.value and eventually the amount would fall under minimum deposit and this conditions could do nothing about it.

    function depositAtom(address receiver, uint256 id) external payable nonReentrant whenNotPaused returns (uint256) {
        if (id == 0 || id > count) {
            revert Errors.MultiVault_VaultDoesNotExist();
        }

        if (isTripleId(id)) {
            revert Errors.MultiVault_VaultNotAtom();
        }

        if (msg.value < generalConfig.minDeposit) {
            revert Errors.MultiVault_MinimumDeposit();
        }

        uint256 protocolFee = protocolFeeAmount(msg.value, id);
        uint256 userDepositAfterprotocolFee = msg.value - protocolFee;

        // deposit eth into vault and mint shares for the receiver
        uint256 shares = _deposit(receiver, id, userDepositAfterprotocolFee);

        _transferFeesToProtocolVault(protocolFee);

        return shares;
    }

Recomendation

• consider moving the protocol fee subtraction to before the if check

• uint256 protocolFee = protocolFeeAmount(msg.value, id);

• uint256 userDepositAfterprotocolFee = msg.value - protocolFee;

• if (userDepositAfterprotocolFee < generalConfig.minDeposit) { revert Errors.MultiVault_MinimumDeposit(); }

• if (msg.value < generalConfig.minDeposit) { revert Errors.MultiVault_MinimumDeposit(); }

• uint256 protocolFee = protocolFeeAmount(msg.value, id);

• uint256 userDepositAfterprotocolFee = msg.value - protocolFee;

**POC**->

- add this function in /test/unit/EthMultiVault/DepositAtom.t.sol
- run with forge test --mt testMinDepositPOC -vvv

```solidity 

function testMinDepositPOC() external {
        // prank call from alice
        // as both msg.sender and tx.origin
        vm.startPrank(alice, alice);

        // test values
        uint256 testAtomCost = getAtomCost();
        uint256 testMinDesposit = getMinDeposit();
        uint256 testDepositAmount = testMinDesposit;

        // execute interaction - create atoms
        uint256 id = ethMultiVault.createAtom{value: testAtomCost}("atom1");

        vm.stopPrank();

        // snapshots before interaction
        uint256 totalAssetsBefore = vaultTotalAssets(id);
        uint256 totalSharesBefore = vaultTotalShares(id);
        console.log("totalAssetsBefore", totalAssetsBefore);
        console.log("totalSharesBefore", totalSharesBefore);
        uint256 protocolVaultBalanceBefore = address(getProtocolVault()).balance;

        vm.startPrank(bob, bob);

        uint256 protocolFee = getProtocolFeeAmount(testDepositAmount, id);
        uint256 valueToDeposit = testDepositAmount - protocolFee;

        uint256 sharesExpected = convertToShares(valueToDeposit - entryFeeAmount(valueToDeposit, id), id);

        // execute interaction - deposit atoms
        ethMultiVault.depositAtom{value: testDepositAmount}(address(1), id);

        checkDepositIntoVault(valueToDeposit, id, totalAssetsBefore, totalSharesBefore);

        checkProtocolVaultBalance(id, testDepositAmount, protocolVaultBalanceBefore);

        (uint256 sharesGot, uint256 assetsGot) = getVaultStateForUser(id, address(1));

        uint256 assetsExpected = convertToAssets(sharesGot, id);

        assertEq(assetsExpected, assetsGot);
        assertEq(sharesExpected, sharesGot);

        //we can see `assetsGot` is less than `minDeposit` value
        assertLt(assetsGot, testDepositAmount);

        console.log("minDeposit: ", testDepositAmount);
        console.log("assetsGot: ", assetsGot);

        vm.stopPrank();
    }

[mihailo-maksa]

The reported issue concerning the potential bypass of the minDeposit logic in ETHmultivault.sol has been reviewed. Here is our detailed response:

Intended Functionality: The minDeposit parameter is designed to enforce a minimum deposit amount based on the initial msg.value provided by the user. This ensures that users deposit a certain minimum amount of ETH, as intended by the protocol.

Deposit Logic: The current implementation checks msg.value against generalConfig.minDeposit before subtracting the protocol fee. This design choice ensures that the initial deposit amount meets the required minimum threshold, regardless of the subsequent fee deduction.

Protocol Design: The logic ensures that the minDeposit requirement applies to the gross deposit amount. This approach simplifies the deposit process and maintains clarity for users, as they only need to ensure their initial deposit meets the minimum requirement without considering fee adjustments.

Conclusion: The existing implementation of the minDeposit check aligns with our protocol design and intent. The minimum deposit requirement is based on the initial deposit (msg.value), not the net amount after fee deductions. Therefore, we consider this issue to be functioning as intended and do not see it as a vulnerability.

Status: This issue is invalid.

[0xarshia]

yes i agree this could be design choice but until its not changed.

the protocolFee is dynamic in protocol. it means the minimum value of deposit also changes and it literally becomes dynamic thing. its can be problematic when third party contracts interacts with protocol.

i don't think this is also design choice otherwise there is no meaning of minDeposit.