the asset to share and share to asset mechanism is erc-4626 incompliant

[hats-bug-reporter[bot]]

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

Description: Description

eip-4626 security considerations (EIP)[https://eips.ethereum.org/EIPS/eip-4626#security-considerations] mentioned the usage of convert functinalities for estimaiting the amount for change needs to be done via time-weighted mechanism implemented inside of it otherwise it would be manipulatable.

This specification includes convert methods that are allowed to be inexact and therefore can be implemented as robust price oracles. For example, it would be correct to implement the convert methods as using a time-weighted average price in converting between assets and shares.

in 'ethmultivault.sol' here is the snippet of convert mechanism used without using time-weighted price and as doc said its more likely to be manipulated,

   function convertToShares(uint256 assets, uint256 id) public view returns (uint256) {
        uint256 supply = vaults[id].totalShares;
        uint256 shares = supply == 0 ? assets : assets.mulDiv(supply, vaults[id].totalAssets);
        return shares;
    }

    function convertToAssets(uint256 shares, uint256 id) public view returns (uint256) {
        uint256 supply = vaults[id].totalShares;
        uint256 assets = supply == 0 ? shares : shares.mulDiv(vaults[id].totalAssets, supply);
        return assets;
    }

    function currentSharePrice(uint256 id) external view returns (uint256) {
        uint256 supply = vaults[id].totalShares;
        uint256 price = supply == 0 ? 0 : (vaults[id].totalAssets * generalConfig.decimalPrecision) / supply;
        return price;
    }

the exchange rate in convert functionalities will be manipulatable by fruntrunning deposit process and many othere ways unles it gives output with the help of time-weighted calculated results

thus it'll cause loss of funds for user

[mihailo-maksa]

The reported issue concerning the compliance of the asset to share and share to asset conversion mechanism with ERC-4626 has been reviewed. Here is our detailed perspective:

Standards Reference: The ERC-4626 standard mentions using time-weighted mechanisms for conversion functions as a security consideration. However, it does not mandate this as a requirement, and the standard itself acknowledges the flexibility in implementation.

Design Considerations: Our implementation of the convertToShares and convertToAssets functions is intentional and designed to be straightforward and efficient. The use of a time-weighted average price (TWAP) is a suggestion for added robustness but not a compulsory feature.

Economic Safeguards: Our protocol includes multiple layers of economic safeguards such as protocolFees, entryFees, and exitFees. These fees significantly reduce the feasibility of any manipulation attempts, making it capital-intensive and likely unprofitable.

Lack of Proof of Concept: The issue report does not provide a Proof of Concept (PoC) or any revised code files to demonstrate the alleged vulnerability. Without concrete evidence or examples of exploit scenarios, the claim remains speculative.

Conclusion: Based on the above points, we do not consider the current implementation to be non-compliant or vulnerable. The absence of time-weighted price mechanisms does not inherently pose a risk due to our economic safeguards. Therefore, we consider this issue to be invalid.

Status: This issue is invalid.