First depositor can break share calculation in ERC4626 valut

[code423n4]

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L331-L335

Vulnerability details

Impact

The first depositor of an ERC4626 vault can maliciously manipulate the share price by depositing the lowest possible amount (1 wei) of liquidity and then artificially inflating assets price.

This can inflate the base share price as high as 1:1e18 early on, which force all subsequence deposit to use this share price as a base and worst case, due to rounding down, if this malicious initial deposit front-run someone else depositing, this depositor will receive 0 shares and lost his deposited assets.

Proof of Concept

Given a vault with DAI as the underlying asset:

Alice (attacker) deposits initial liquidity of 1 wei DAI via deposit()
Alice receives 1e18 (1 wei) vault shares
Alice transfers 1 ether of DAI via transfer() to the vault to artificially inflate the asset balance without minting new shares. The asset balance is now 1 ether + 1 wei DAI -> vault share price is now very high (= 1000000000000000000001 wei ~ 1000 * 1e18)
Bob (victim) deposits 100 ether DAI
Bob receives 0 shares
Bob receives 0 shares due to a precision issue. His deposited funds are lost.

The shares are calculated as following
if (totalSupply() == 0) { shares = _amount; } else { shares = (_amount * totalSupply()) / freeFunds; } _mint(_receiver, shares);

ERC4626 vault share price can be maliciously inflated on the initial deposit, leading to the next depositor losing assets due to precision issues.

Tools Used

Manual Review

Recommended Mitigation Steps

Uniswap V2 solved this problem by sending the first 1000 LP tokens to the zero address. The same can be done in this case i.e. when totalSupply() == 0, send the first min liquidity LP tokens to the zero address to enable share dilution.

In _deposit(), ensure the number of shares to be minted is non-zero:
require(shares != 0, "No shares minted");

[c4-judge]

trust1995 marked the issue as duplicate of #848

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 changed the severity to QA (Quality Assurance)

[c4-judge]

trust1995 marked the issue as grade-b

[c4-sponsor]

0xBebis marked the issue as sponsor acknowledged