First depositor can break minting of shares

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/Vault.sol#L294-L316

Vulnerability details

Vulnerability details

The calculation of exchange rate for shares in Popcorn Vault is done by dividing the total supply of shares by the totalAssets of the vault. The first depositor can mint a very small number of shares, then donate to the vault to manipulate the share price. When subsequent depositors deposit, they will lose value due to precision loss. This is a common attack vector for almost all shares based liquidity pool contracts using ERC4626.

Impact

First depositor can manipulate shares from later users; later users will not get equivalent shares when converting their underlying asset.

Proof of Concept

1. Malicious user Alice can deposit() with 1 wei of asset token to get 1 wei of shares.

2. Next, Alice sends 10000e18 -1 of asset tokens and inflate the price per share from 1 to 1e22.

3. Subsequent depositor who deposits shares, eg 19999e18 of assets, will only receive 1 wei of shares token.

4. Victim will lose 9999e18 if they redeem() right after deposit() due to precision loss.

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/Vault.sol#L294-L316

Relatable issue:

https://github.com/sherlock-audit/2022-08-sentiment-judging#issue-h-1-a-malicious-early-userattacker-can-manipulate-the-ltokens-pricepershare-to-take-an-unfair-share-of-future-users-deposits

Tools Used

Manual Review

Recommended Mitigation Steps

Consider requiring a minimum amount of share tokens to be minted for the first minter or follow Uniswap V2 which mints 10,000 share first to balance liquidity whenever someone wants to open a vault.

https://github.com/Uniswap/v2-core/blob/ee547b17853e71ed4e0101ccfd52e70d5acded58/contracts/UniswapV2Pair.sol#L119-L124

[c4-judge]

dmvt marked the issue as duplicate of #15

[c4-sponsor]

RedVeil marked the issue as sponsor confirmed

[c4-judge]

dmvt marked the issue as satisfactory

[c4-judge]

dmvt marked the issue as partial-50

[c4-judge]

dmvt marked the issue as satisfactory