Liquid Staking

Summary

An arbitrary external user can influence the frequency of vault upgrades.

Vulnerability Details

Inbound deposits to a VaultControllerStrategy must unconditionally always ensure the first vault they deposit to is equal to the groupDepositIndex:

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/base/VaultControllerStrategy.sol#L183C9-L185C38

However, each successful deposit groupDepositIndex also configures the next groupDepositIndex.

Consequently, a caller can append an arbitrary vaultId to the end of their array of vaultIds to extert control over the groupDepositIndex.

Impact

Depositing into a VaultControllerStrategy is permissionless.

1. Through negligible donations, a user can force their vault to receive more deposits on average by forcing depositors to include their chosen vault by prioritising the groupDepositIndex to match their chosen vault.

2. If _vaultIds[0] does not start at groupDepositIndex, the call is reverted. This means an attacker can therefore cheaply modify the groupDepositIndex as a means to DoS inbound deposits.

Tools Used

Manual Review

Recommendations

1. Do not enforce which vaults should be deposited into based upon conditions which can be influenced by untrusted parties.

2. Always ensure a nontrivial minimum deposit is returned via getVaultDepositLimits(), never allow this to be 0.