Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

Risk of overflow leading to a denial of service(DoS) in the `CommunityVCS::deposit` function.

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 vault deposit limit has changed in Chainlink staking contract, make adjustments
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;
}
// @audit risk of overflow if the value of (numVaults * diff) exceeds the range of uint128
@> vaultGroups[i].totalDepositRoom += uint128(numVaults * diff);
}
vaultMaxDeposits = maxDeposits;
}
/// ... The rest of code
}

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.
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.