First depositor can break minting of shares

[nemveer]

Title

First depositor can break minting of shares

Affected smart contract

BaseVault.sol All vaults that inherit BaseVault.sol

Description

When totalSupply is zero an attacker goes ahead and executes the following steps

1. They deposit 1 wei of underlying token. https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/abstracts/BaseVault.sol#L307-L321
   • Which eventually calls the _convertToShares() to determine the shares to mint
   • They will get back 1 wei of overlyingToken because totalSupply is zero
2. They transfer z underlying tokens directly to vault address
   • This leads to 1 wei of overlyingToken worth z (+ some small amount)
   • Attacker won't have any problem making this z as big as possible as they have all the claim to it as a holder of 1 Wei of overlyingToken

Impact

1. The first deposit can be front-run and stolen
   • Let's assume there is a first user trying to mint some overlyingToken using their k*z underlying tokens.
   • An attacker can see this transaction and carry out the above-described attack making sure that k<1.
   • This leads to the first depositor getting zero overlyingToken for their k*z underlying tokens. Thus 100% loss of funds(Now the underlying reserve of the contract is also increased with each deposit, so the upcoming depositor will have to deposit an even greater amount to mint some token)
   • All upcoming depositors will face the same.
   • All the tokens are redeemable by the attacker using their 1 wei of overlyingToken.
2. Implicit minimum Amount and funds lost due to rounding errors
   • If an attacker is successful in making 1 wei of overlyingToken worth z underlying tokens and a user tries to mint overlyingToken using k* z underlying tokens then,
   • If k<1, then the user gets zero overlyingToken and all of their underlying tokens get proportionally divided between overlyingToken holders
     • This leads to an implicit minimum amount for a user at the attacker's discretion.
   • If k>1, then users still get some overlyingToken but they lose (k- floor(k)) * z) of underlying tokens which get proportionally divided between overlyingToken holders due to rounding errors.
   • Attacker can keep 2 wei instead of 1 too. But that would require double the capital. But then the attack would be more powerful.

Recommendation

• For already deployed contracts send some sToken to a dead address to make sure that the total supply never goes to zero
• For new contracts, I like how BalancerV2 and UniswapV2 do it, some minimum amount of pool tokens get burnt when the first mint happens.

[0xdcota]

This issue will closed in PR #498.