#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

• Report ID: #42604

• Report Type: Smart Contract

• Report severity: Low

• Target: https://github.com/immunefi-team/audit-comp-yeet/blob/main/src/contracts/MoneyBrinter.sol

• 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.

function previewWithdraw(uint256 assets) public view override returns (uint256) {
    uint256 fee = _feeOnRaw(assets, exitFeeBasisPoints);
    return super.previewWithdraw(assets + fee);
}

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.

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 - https://eips.ethereum.org/EIPS/eip-4626#maxWithdraw

Proof of Concept

Proof of Concept

The following test could be added to the VaultWithdrawTest.t test file.

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.