# #42604 \[SC-Low] \`MoneyBrinter\` vault does not conform to ERC4626 **Submitted on Mar 24th 2025 at 23:12:58 UTC by @NHristov for** [**Audit Comp | Yeet**](https://immunefi.com/audit-competition/audit-comp-yeet) * **Report ID:** #42604 * **Report Type:** Smart Contract * **Report severity:** Low * **Target:** * **Impacts:** * Contract fails to deliver promised returns, but doesn't lose value ## Description ## Brief/Intro Vault’s implementation of ERC4626 returns an incorrect value in `maxWithdraw` when exit fees are applied. If a user attempts to withdraw using the maxWithdraw value, the transaction will revert, which means that the vault does not fully conform to the expected ERC4626 standard. ## Vulnerability Details The `ERC4626 specification` states that the `maxWithdraw` function `MUST return the maximum amount of assets that could be transferred from owner through withdraw and not cause a revert, which MUST NOT be higher than the actual maximum that would be accepted (it should underestimate if necessary).` but the current `MoneyBrinter` contract does not conform this statement when fees are applied on withdraw. The calculation of the fees can be found in the following `MoneyBrinter` functions. ```solidity function previewWithdraw(uint256 assets) public view override returns (uint256) { uint256 fee = _feeOnRaw(assets, exitFeeBasisPoints); return super.previewWithdraw(assets + fee); } ``` ```solidity function _feeOnRaw(uint256 assets, uint256 feeBasisPoints) private pure returns (uint256) { return assets.mulDiv(feeBasisPoints, _BASIS_POINT_SCALE, Math.Rounding.Ceil); } ``` but the `maxWithdraw` function is not overriden by the default implementation of the ERC4626 Openzeppelin contract. ```solidity function maxWithdraw(address owner) public view virtual returns (uint256) { return _convertToAssets(balanceOf(owner), Math.Rounding.Floor); } ``` This shows that `maxWithdraw` produces an amount that cannot be successfully withdrawn due to the fee calculation, and thus the vault fails to meet the requirements of ERC4626 and leads to transactions being reverted. ## Impact Details `MoneyBrinter` does not conform to ERC4626 which may break external integrations and leading to failed transactions. ## References ERC4626 documentation - ## Proof of Concept ## Proof of Concept The following test could be added to the `VaultWithdrawTest.t` test file. ```solidity function testMaxWithdrawReturnsIncorectValueWhenFeeIsApplied() public { uint256 feeBps = 1000; // 1% _setExitFee(feeBps); uint256 withdrawAmount = vault.maxWithdraw(bob); // we expect to revert with ERC4626ExceededMaxWithdraw vm.expectRevert(); vm.prank(bob); vault.withdraw(withdrawAmount, bob, bob); } ``` this can be executed by running the command: ``` forge test --mt testMaxWithdrawReturnsIncorectValueWhenFeeIsApplied -vvvv ``` Here, even though `vault.maxWithdraw(bob)` returns 100 tokens, this amount does not account for the exit fee. When the user calls `vault.withdraw` with this amount, the fee is added on, causing the withdrawal to exceed the available balance, which ultimately reverts the transaction. --- # Agent Instructions: Querying This Documentation If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question. Perform an HTTP GET request on the current page URL with the `ask` query parameter: ``` GET https://reports.immunefi.com/yeet/42604-sc-low-moneybrinter-vault-does-not-conform-to-erc4626.md?ask= ``` The question should be specific, self-contained, and written in natural language. The response will contain a direct answer to the question and relevant excerpts and sources from the documentation. Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.