First depositor can partially steal deposits and DoS vaults

[rokinot]

Issue: #392

Severity

High

Description

This issue contains code logic from both HATVault.sol and ERC4626Upgradeable.sol which is an abstract contract inherited by HATVault.sol. The first vault's depositor is able to compromise vaults by minting a minimum amount of shares (such as 1 wei of the staking token) and transferring a much larger amount further on (1 ether worth of staking tokens, or 1e18). This makes it so supply in ERC4626Upgradeable.sol will be 1, while totalAssets() will return 1e18. This makes the share price extremely overvalued, and every future deposit below the transferred amount will revert due to convertToShares() returning 0.

Deposit with amounts above this amount transferred by the malicious actor will proceed, however due to rounding issues, the depositor . In the example below, the code has the first malicious actor transferring 1e18 stakingToken amount, a second clueless actor depositing 1.5e18 tokens will only receive 1 share. Since the total token amount is 2.5e18 tokens, 1 share will be able to withdraw 1.25e18 tokens, so the first depositor can exit with a stolen profit of 0.25e18.

Mitigation

Uniswap V2 faced the same vulnerability, and fixed it by automatically minting 1000 shares and sending it to address zero. You can check the code snippet here. For this project, I recommend sending those 1000 shares to address(0) in the HATVault.sol initializer.

[CLAassistant]

All committers have signed the CLA.