FIRST ERC4626 DEPOSIT CAN BE EXPLOITED ON SHARE CALCULATION

[code423n4]

Lines of code

https://github.com/AstariaXYZ/astaria-gpl/blob/4b49fe993d9b807fe68b3421ee7f2fe91267c9ef/src/ERC4626-Cloned.sol#L107-L113

Vulnerability details

Impact

This is a common attack vector involving shares based liquidity pool contracts. An early user can manipulate the price per share and profit from late users' deposits because of the precision loss caused by the rather large value of price per share.
(Note: In the case of the protocol, the price per share relates to the amount of fund (asset) tokens per share.)

ERC4626-Cloned.sol#L107-L113

  function convertToShares(
    uint256 assets
  ) public view virtual returns (uint256) {
    uint256 supply = totalSupply(); // Saves an extra SLOAD if totalSupply is non-zero.

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

Proof of Concept

Here is the exploit scenario:

1. A malicious early user can call deposit() with 1 wei of asset token as the first depositor, and gets 1 wei of shares as is evidenced in the ternary return statement above.

2. Next, the attacker will send 10000e18 - 1 of asset tokens and inflate the price per share from 1.0000 to an extreme value of 1.0000e22 ( from (1 + 10000e18 - 1) / 1).

3. Consequently, the future user who deposits 19999e18 will only receive 1 wei (from 19999e18 * 1 / 10000e18) of shares token.

4. He/she will immediately lose 9999e18 or equivalently half of the deposits if redeem() is called right after deposit(), albeit to be realized in the next epoch.

Conclusion: The attacker can profit from future users' deposits whilst the late users will lose part of their funds to the attacker.

Recommended Mitigation Steps

It is recommended sending the first 1000 shares to address 0, a mitigation approach adopted by the Uniswap V2 protocol.

Additionally, the protocol should strongly advise depositors to deposit funds via AstariaRouter.sol (instead of PublicVault.sol to avoid this specific leak) that will have a slippage protection.

[c4-judge]

Picodes marked the issue as duplicate of #509

[c4-judge]

Picodes marked the issue as duplicate of #588

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 3 (High Risk)