First PublicVault depositor can be front-run and have part of their deposit stolen

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/main/src/PublicVault.sol#L251-L265
https://github.com/AstariaXYZ/astaria-gpl/blob/4b49fe993d9b807fe68b3421ee7f2fe91267c9ef/src/ERC4626-Cloned.sol#L112

Vulnerability details

Description

The first deposit with a totalSupply of zero shares will mint shares equal to the deposited amount.

File: lib/gpl/src/ERC4626-Cloned.sol

112:     return supply == 0 ? assets : assets.mulDivDown(supply, totalAssets());

Link to Code

File: PublicVault.sol

    function deposit(uint256 amount, address receiver)
      public
      override(ERC4626Cloned)
      whenNotPaused
      returns (uint256)
    {
      VIData storage s = _loadVISlot();
      if (s.allowListEnabled) {
        require(s.allowList[receiver]);
      }

      uint256 assets = totalAssets();

      return super.deposit(amount, receiver);
    }

Link to Code

Problems with the code:

1. Integer division negatively affect the user.
2. Can be manipulated to cause a large loss, specifically for first victim.

Impact

It can lead to some part of Fund getting stolen from First Depositor (which will be the LP Provider).

Proof of Concept

Consider the following situation:

1. Attacker deposits 1 wei of WETH.
2. Next, Attacker transfers 100 WETH to the contract.
3. Victim deposits 200 WETH.
4. Attacker withdraws 1 share.

Here is the Detail analysis of the above PoC done by Spearbit.

This analysis confirms this clear Path of attack which can be used by the attacker.

Tools Used

Manual Review

Recommended Mitigation Steps

1. Need to Enforce a minimum deposit that can't be withdrawn.
2. So, mint some of the initial amount to the zero address.
3. Most legit first depositors will mint thousands of shares, so not a big cost.

[c4-judge]

Picodes marked the issue as primary issue

[c4-judge]

Picodes marked the issue as duplicate of #588

[c4-judge]

Picodes marked the issue as satisfactory