Summary
The CommunityVCS contract initializes the depositIndex field of the GlobalVaultState struct using a value derived from maxDepositSizeBP, which is conceptually incorrect. The depositIndex should represent the index of the next non-group vault to receive deposits, and it should be initialized based on the current number of deployed vaults if address (token) != address (0).
Vulnerability Details
In the CommunityVCS contract, the globalVaultState is initialized as follows:
function initialize(
address _token,
address _stakingPool,
address _stakeController,
address _vaultImplementation,
Fee[] memory _fees,
uint256 _maxDepositSizeBP,
uint256 _vaultMaxDeposits,
uint128 _vaultDeploymentThreshold,
uint128 _vaultDeploymentAmount,
address _vaultDepositController
) public initializer {
if (address(token) == address(0)) {
__VaultControllerStrategy_init(
_token,
_stakingPool,
_stakeController,
_vaultImplementation,
_fees,
_maxDepositSizeBP,
_vaultMaxDeposits,
_vaultDepositController
);
vaultDeploymentThreshold = _vaultDeploymentThreshold;
vaultDeploymentAmount = _vaultDeploymentAmount;
_deployVaults(_vaultDeploymentAmount);
globalVaultState = GlobalVaultState(5, 0, 0, 0);
} else {
globalVaultState = GlobalVaultState(5, 0, 0, uint64(maxDepositSizeBP + 1));
maxDepositSizeBP = _maxDepositSizeBP;
delete fundFlowController;
vaultMaxDeposits = _vaultMaxDeposits;
}
for (uint64 i = 0; i < 5; ++i) {
vaultGroups.push(VaultGroup(i, 0));
}
}
This is also seen in the operatorVCS contract as shown below:
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/OperatorVCS.sol#L52-L84
function initialize(
address _token,
address _stakingPool,
address _stakeController,
address _vaultImplementation,
Fee[] memory _fees,
uint256 _maxDepositSizeBP,
uint256 _vaultMaxDeposits,
uint256 _operatorRewardPercentage,
address _vaultDepositController
) public reinitializer(3) {
if (address(token) == address(0)) {
__VaultControllerStrategy_init(
_token,
_stakingPool,
_stakeController,
_vaultImplementation,
_fees,
_maxDepositSizeBP,
_vaultMaxDeposits,
_vaultDepositController
);
if (_operatorRewardPercentage > 10000) revert InvalidPercentage();
operatorRewardPercentage = _operatorRewardPercentage;
globalVaultState = GlobalVaultState(5, 0, 0, 0);
} else {
globalVaultState = GlobalVaultState(5, 0, 0, uint64(maxDepositSizeBP + 1));
maxDepositSizeBP = _maxDepositSizeBP;
delete fundFlowController;
vaultMaxDeposits = _vaultMaxDeposits;
}
The depositIndex of the struct GlobalVaultState is set to uint64(maxDepositSizeBP + 1), which is incorrect because maxDepositSizeBP is related to deposit size limits in basis points, not vault indexing. This could lead to incorrect deposit behavior, as the depositIndex is used to track where the next deposit should occur.
Impact
The incorrect initialization of depositIndex can lead to deposits being directed to the wrong vaults, potentially causing deposit distribution and management issues. This could affect the vault system's overall functionality and efficiency.
Tools Used
Recommendations
Update the initialization of globalVaultState to use the correct value for depositIndex. It should be based on the index of the next non-group vault to receive deposits