Vaults are vulnerable to inflation attacks

[code423n4]

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L925-L963 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L870-L873

Vulnerability details

Impact

The first depositor can be front run by an attacker and as a result, will lose a considerable part of the assets provided.

Proof of Concept

The vault calculates the amount of shares to be minted upon deposit to every user via the convertToShares function.

When the pool has no share supply, the amount of shares to be minted is equal to the assets provided. An attacker can abuse this situation and profit from the rounding down operation when calculating the amount of shares if the supply is non-zero:

return
  (_assets == 0 || _exchangeRate == 0)
    ? _assets
    : _assets.mulDiv(_assetUnit, _exchangeRate, _rounding);

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L870-L873

This issue can be replicated with the following steps:

1. Alice wants to deposit 2M USDC into the vault
2. Bob frontruns Alice and he deposits 1 wei, thus he receives 1 share
3. Bob makes a "donation" to increase the exchange rate, transferring a total of 1M USDC
4. Alice's transaction is executed, but she receives only 1 share with the new supply, as the division rounds down
5. Bob backruns Alice and he redeems his share, receiving a total of 1.5M USDC
6. Alice has lost 50% of her funds

Tools Used

Manual review

Recommended Mitigation Steps

Require a minimum amount of initial shares to be minted and/or send part of the initial LP to the zero address.

Assessed type

ERC4626

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid

[Picodes]

Donation would lead to the next depositor not having to bring assets in this case

[Picodes]

I think the donation attack may work if done in the yield vault token, but it's not the scenario here

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked the issue as duplicate of #341

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[c4-judge]

Picodes marked the issue as partial-50

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid