Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

The `vaultGroups` storage variable is not updated after the loop, which will result in a loss of values in the `VaultDepositController::_depositToVaults` function.

0xrolko

Relevant GitHub Links

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

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

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

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

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

Summary

Loss of value after update vaultGroups array in the _depositToVaults function.

Vulnerability Details

While depositing token into the `vaults` with the `VaultDepositController::_depositToVaults` function, the `group` is being fetched from `vaultGroups` at the start of the loop. Some properties of the `group` are then modified (such as `withdrawalIndex` and `totalDepositRoom`) during the loop. However, *after modifying `group`*, it is stored only in the local memory, and not reflected back to the `vaultGroups` state array. Meaning that the changes made to group will not persist beyond the loop:

function _depositToVaults(
uint256 _toDeposit,
uint256 _minDeposits,
uint256 _maxDeposits,
uint64[] memory _vaultIds
) private returns (uint256) {
uint256 toDeposit = _toDeposit;
uint256 totalRebonded;
GlobalVaultState memory globalState = globalVaultState;
@> VaultGroup[] memory groups = vaultGroups;
​
// deposits must continue with the vault they left off at during the previous call
if (_vaultIds.length != 0 && _vaultIds[0] != globalState.groupDepositIndex)
revert InvalidVaultIds();
​
// deposit into vaults in the order specified in _vaultIds
for (uint256 i = 0; i < _vaultIds.length; ++i) {
/// ... The other code
@> VaultGroup memory group = groups[groupIndex];
uint256 deposits = vault.getPrincipalDeposits();
uint256 canDeposit = _maxDeposits - deposits;
​
globalState.groupDepositIndex = uint64(vaultIndex);
​
// if vault is empty and equal to withdrawal index, increment withdrawal index to the next vault in the group
if (deposits == 0 && vaultIndex == group.withdrawalIndex) {
@> group.withdrawalIndex += uint64(globalState.numVaultGroups);
if (group.withdrawalIndex > globalState.depositIndex) {
@> group.withdrawalIndex = uint64(groupIndex);
}
}
​
if (canDeposit != 0 && vaultIndex != group.withdrawalIndex && !vault.isRemoved()) {
/// ... The other code
​
if (toDeposit > canDeposit) {
vault.deposit(canDeposit);
toDeposit -= canDeposit;
@> group.totalDepositRoom -= uint128(canDeposit);
} else {
vault.deposit(toDeposit);
@> group.totalDepositRoom -= uint128(toDeposit);
toDeposit = 0;
break;
}
}
// @audit : the updated group values will be lost after the loop
//vaultGroups[groupIndex] = group; // Ensure group state is updated
}
​
/// ... The rest of code
}

Impact

The updated group values will be lost after the loop finishes because they are only changed in memory, not in the contract's persistent storage.

Tools Used

Manual review.

Recommendations

function _depositToVaults(
uint256 _toDeposit,
uint256 _minDeposits,
uint256 _maxDeposits,
uint64[] memory _vaultIds
) private returns (uint256) {
​
/// ... The rest of code
​
for (uint256 i = 0; i < _vaultIds.length; ++i) {
/// ... The other code
​
+ vaultGroups[groupIndex] = group; // Ensure group state is updated
}
​
/// ... The rest of code
}
Updates

Lead Judging Commences

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

Support

FAQs

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