Relevant GitHub Links
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/CommunityVCS.sol#L90
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/CommunityVCS.sol#L97
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/CommunityVCS.sol#L102
Summary
A loss of value will occur in `CommunityVCS::deposit` function when the range of `uint128` is exceeded.
Vulnerability Details
When tokens from the staking pool are deposited in vaults using the CommunityVCS::deposit function, the struct VaultGroup::totalDepositRoom variable is calculated. However, this variable is of type uint128 and the two variables used to calculate it are of type uint256, so a cast is made to convert the result from uint256 to uint128. However, the uint128 type cannot contain all the values of uint256. type(uint256).max = 2**256 -1 and type(uint128).max =2**128-1 so each time the result of the calculation of the totalDepositRoom variable exceeds the capacity of the uint128 type, the cast will truncate the most significant bits that do not fit within the size of `uint128`, resulting in a loss of data. So the deposit of the right amount is really successful only when the result of the calculation of the `totalDepositRoom` variable is less than or equal to `type(uint128).max`:
function deposit(uint256 _amount, bytes calldata _data) external override onlyStakingPool {
(, uint256 maxDeposits) = getVaultDepositLimits();
if (maxDeposits > vaultMaxDeposits) {
@> uint256 diff = maxDeposits - vaultMaxDeposits;
uint256 totalVaults = globalVaultState.depositIndex;
uint256 numVaultGroups = globalVaultState.numVaultGroups;
uint256 vaultsPerGroup = totalVaults / numVaultGroups;
uint256 remainder = totalVaults % numVaultGroups;
for (uint256 i = 0; i < numVaultGroups; ++i) {
@> uint256 numVaults = vaultsPerGroup;
if (i < remainder) {
numVaults += 1;
}
@> vaultGroups[i].totalDepositRoom += uint128(numVaults * diff);
}
vaultMaxDeposits = maxDeposits;
}
}
Impact
Loss of value when `(numVaults * diff)` exceeds the range of `uint128` type.
Tools Used
Manual review.
Recommendations
function deposit(uint256 _amount, bytes calldata _data) external override onlyStakingPool {
(, uint256 maxDeposits) = getVaultDepositLimits();
// if vault deposit limit has changed in Chainlink staking contract, make adjustments
if (maxDeposits > vaultMaxDeposits) {
/// ... The rest of code.
for (uint256 i = 0; i < numVaultGroups; ++i) {
uint256 numVaults = vaultsPerGroup;
if (i < remainder) {
numVaults += 1;
}
+ require(numVaults * diff <= type(uint128).max, "Value exceeds uint128 range");
vaultGroups[i].totalDepositRoom += uint128(numVaults * diff);
}
vaultMaxDeposits = maxDeposits;
}
/// ... The rest of code.
}