Vault deposit are not checked with maxDeposit() due to missing functionality

[hats-bug-reporter]

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

Description:
Description\

Based on Intuition documentation, EthMultiVault.sol conforms to EIP-4626 standard.

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.

This issue is with deposit() functions like depositAtom() and depositTriple() functions. There is no maximum deposit function in EthMultiVault.sol contract but there is maxRedeem() function which is incompliance with EIP-4626.

maxDeposit() is requirement of EIP4626 which is used as a maximum amount of the underlying asset that can be deposited into the Vault for the receiver, through a deposit call i.e with depositAtom() and depositTriple() functions.

Recommendation to fix\

Consider implementing maxDeposit() function and check the receiver deposits in both depositAtom() and depositTriple().

[mihailo-maksa]

The issue highlights the absence of the maxDeposit function in EthMultiVault.sol, which is required by the EIP-4626 standard.

Label: invalid

Comment:
Since the standard return value of maxDeposit() is typically type(uint256).max, we originally omitted this check for gas efficiency reasons. The suggested improvement aligns with ERC-4626 compatibility but seems unnecessary and inefficient in our context as we do not intend to limit deposit amounts artificially.

Additional Comment:
This check is largely redundant as we do not intend to limit the amount of ETH users can deposit, and no practical scenario would trigger a revert due to maxDeposit. While we might add maxDeposit as a view function for completeness, we likely won't implement the check inside depositTriple and depositAtom due to gas inefficiency.

Comment on the issue: We initially omitted maxDeposit for gas efficiency. While technically more ERC-4626 compatible, implementing this check seems unnecessary. We might add maxDeposit as a view function but won't add the check in depositTriple and depositAtom due to gas inefficiency.

[0xRizwan]

@mihailo-maksa I understand your point but EthMultiVault.sol conforms to EIP-4626 standard. maxDeposit() must be implemented in contract. I see, maxRedeem() is implemented so there is inconsistency with max deposit and max withdraw in contract and also not implementing maxDeposit() is non-compliance of EIP-4626 specifications. I believe, this is valid low issue.

[mihailo-maksa]

Here is our detailed perspective:

• This issue doesn’t meet any of the criteria for acceptance, since the omission of maxDeposit() does not compromise the security or functionality as per our bug bounty program's scope.
• Unlike issue Minor Inefficiencies in _deposit Function #72, which was accepted due to its utility and implementation, this issue is not critical to the protocol's operation and will not be addressed.

Scope of the Bug Bounty Program

The scope includes the core contracts of the Intuition protocol:

• EthMultiVault.sol
• AtomWallet.sol

Examples of What's in Scope

• Stealing or freezing user assets (ETH)
• Unauthorized pausing or unpausing of the protocol
• Internal accounting errors leading to incorrect fee charges or asset distribution
• Unauthorized changes to contract configuration

Out of Scope

• Non-security-related suggestions (we believe your issue lies here)
• Efficiency improvements
• Known weaknesses documented in audits

Severity Levels (examples)

• High/Critical: Unauthorized manipulation, theft, freezing of funds
• Medium: Exploits compromising user experience
• Low: Non-critical functionality issues without user fund loss

Please refer to the readme file for more details on intended behavior and the developer docs.