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

Deposit limit should always be changed if different from Chainlink, however this is not done

bauchibred

Summary

Deposits are not changed even when they are supposed to.

Vulnerability Details

Take a look at https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/CommunityVCS.sol#L85-L115

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;
}
​
vaultGroups[i].totalDepositRoom += uint128(numVaults * diff);
}
​
vaultMaxDeposits = maxDeposits;
}
​
if (vaultDepositController == address(0))
revert VaultDepositControllerNotSet();
​
(bool success, ) = vaultDepositController.delegatecall(
abi.encodeWithSelector(
VaultDepositController.deposit.selector,
_amount,
_data
)
);
​
if (!success) revert DepositFailed();
}

This function is used to deposit tokens from the staking pool into vaults. It includes logic that makes adjustments if the vault deposit limit has changed in the Chainlink staking contract. The issue, however, is that this functionality does not take into account that the max deposit could be decreased and not increased.

From 7.2.2 in the September Codehawks report, we can see how Cyfrin has advised that the vaultMaxDeposits should be updated once all vault group total deposit rooms are updated. However, this is not being respected.

Impact

Accounting for the totalDepositRoom would be inaccurate. If the max is decreased by Chainlink, then we should have a lower room for deposit, but that's not being followed, which would then mean the system attempts to process deposits even if it should not.

Tools Used

Manual review

Recommendations

Change > to !=. And if in any instance this value from Chainlink via getVaultDepositLimits() is reduced and lower than maxDeposits.

Citations:

Updates

Lead Judging Commences

inallhonesty Lead Judge
9 months ago
inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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