Code is vulnerable to ERC4626 first depositor shares manipulation exploit

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/9e9bb60f117334da7c5d851646a168ca271575fc/src/vaults/PirexERC4626.sol#L80

Vulnerability details

Proof of Concept

The normal ERC4626 implementation (which is not changed in the repository) has a vulnerability which can result in the first depositor stealing every subsequent depositor’s funds.

It works like this:

1. Vault is just deployed and Bob deposits just 1 wei of underlying, so he now holds 1 share
2. Alice is about to deposit 1000 * 1e18 worth of underlying
3. Bob sees this in the mempool and front runs her transaction with a direct ERC20::transfer to the vault for 1000 * 1e18 tokens
4. Now her deposit will result in 0 shares, because the amount she deposited was less than the balance of the contract
5. Now Bob backruns her deposit by redeeming the whole underlying balance for his 1 share (the total supply) resulting in him stealing all of Alice’s deposited tokens

Impact

This can result in a 100% loss of deposited funds for users of the protocol, so it should be of High severity.

Recommendation

Revert when the shares minted are zero.

[c4-judge]

Picodes marked the issue as duplicate of #407

[c4-judge]

Picodes marked the issue as satisfactory

[C4-Staff]

JeeberC4 marked the issue as duplicate of #275