inspecktor - Inflation share price by first depositor in LMPVault.sol

[sherlock-admin2]

inspecktor

false

Inflation share price by first depositor in LMPVault.sol

Summary

Inflation share price can be done by depositing as soon as the vault is created.

Impact:

1. Early depositor will be able to steal other depositors funds
2. Exchange rate is inflated. As a result, depositors are not able to deposit small funds.

The problem exists because the exchange rate is calculated as the ratio between shares totalSupply and totalAssets().
When an attacker transfers assets, totalAssets() incrementally increases and hence the exchange rate also increases.

ERC4626 vaults are subject to a share price manipulation attack that allows an attacker to steal the underlying tokens from other contributors (this is a known issue with the Solmate ERC4626 implementation (transmissions11/solmate#178)).

Vulnerability Detail

1. Alice - the first storekeeper;

2. Alice contributes 1 wei tokens;

3. In the deposit() function, the number of shares is calculated using the previewDeposit() function:
   function deposit(
   uint256 assets,
   address receiver
   ) public virtual override nonReentrant noNavChange ensureNoNavOps returns (uint256 shares) {
   Errors.verifyNotZero(assets, "assets");
   if (assets > maxDeposit(receiver)) {
   revert ERC4626DepositExceedsMax(assets, maxDeposit(receiver));
   }

   shares = previewDeposit(assets);

   _transferAndMint(assets, shares, receiver);
   }

4. Since Alice is the first contributor (totalSupply is 0), she gets 1 share (1 wei);

5. Then Alice sends 9999999999999999999 tokens (10e18 - 1) to the vault;

6. The price for 1 share is now 10 tokens: Alice is the only depositor in the vault, she holds 1 wei of shares, and the pool balance is 10 tokens;

7. Bob contributes 19 tokens and only gets 1 share due to rounding in the convertToShares function: 19e18 * 1 / 10e18 == 1;

8. Alice redeems her share and receives half of the deposited assets, 14.5 tokens (minus the withdrawal fee);

9. Bob redeems his share and receives only 14.5 tokens (minus withdrawal fees) instead of the 19 tokens he deposited.

Tool used

Manual Review

Recommendation

Consider any of these options:

1. In the deposit function, consider requiring a fairly high minimum amount of assets on your first deposit. The amount should be high enough to mint many shares to reduce rounding error, and low enough to be accessible to users.
2. When making your first deposit, consider issuing a fixed and large number of shares, regardless of the deposit amount.
3. Consider filling up pools during deployment. This must be done in deployment transactions to avoid preemptive attacks. The sum must be high enough to reduce the rounding error.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

Trumpero commented:

invalid. totalAsset = totalIdle + totalDebt, totalAsset doesn't use balance of contract. Any deposit will increase totalIdle, so first depositor can't inflate shares of LMPVault