Liquid Staking

Summary

The StakingPool::deposit() function incorrectly assumes that all tokens sent for staking are fully deposited into strategies. When certain scenarios prevent all tokens from being deposited, the excess tokens are returned to the StakingPool, but the protocol fails to properly account for these tokens, leading to the minting of excess LST and an incorrect totalStaked value. This issue can arise due to decreased vault deposit limits or outdated strategy::canDeposit values, and the post-condition intended to prevent this is flawed, allowing the error to persist.

Vulnerability Details

During the deposit process, the StakingPool::_depositLiquidity() function calls strategy::deposit() to deposit tokens into the staking strategy. However, there are scenarios where the entire amount of tokens cannot be deposited:

1. Vault Deposit Limit Reduction:
   The CommunityVC::deposit() function updates vaultMaxDeposits based on the Chainlink staking contract's vault deposit limits. If these limits are decreased, fewer tokens can be deposited than anticipated. The function only handles the case where the limits are increased but fails to account for decreases, leading to excess tokens being returned to the StakingPool. These excess tokens are not handled correctly, causing an overestimation of the totalStaked amount.

   • Vault deposit limits update

2. Outdated strategy::canDeposit Value:
   The strategy::canDeposit() function returns the available deposit room by calculating the difference between the maximum deposit and the current totalDeposits. totalDeposits is periodically updated to reflect the real state of the contract. If the StakingPool::_depositLiquidity function calls this before the totalDeposits value is updated, the protocol may send more tokens than can actually be deposited. The excess tokens are returned to the StakingPool, leading to incorrect accounting.

   • Outdated canDeposit value

In both cases, excess tokens are sent back to the StakingPool, as shown in the VaultControllerStrategy contract:

Ideally, the protocol should revert when the strategy cannot deposit all the tokens, as enforced by the following post-condition in StakingPool::deposit:

However, this check is flawed. It only reverts if both conditions are true, meaning if the returned tokens are not large enough to satisfy the second condition, the deposit proceeds even though the protocol failed to deposit all the tokens. As a result, the totalStaked is inflated, and excess LST is minted.

You can view the relevant code here:

• Excess tokens handling

• Flawed post-condition in StakingPool::deposit

Impact

This issue results in the protocol minting excess LST and recording an inaccurate totalStaked value, which is higher than the actual staked amount. When the excess tokens are finally deposited into the staking strategies, they are double-counted, leading to an imbalance in the staking pool. This ultimately undermines the integrity of the staking system and can cause discrepancies in user rewards and protocol accounting.

Tools Used

Manual

Recommendations

• Ensure that the vaultMaxDeposits value is updated correctly when the vault deposit limit is decreased in the Chainlink staking contract.

• Guarantee that strategy::canDeposit() always returns the correct and updated value before depositing tokens.

• Modify the post-condition in StakingPool::deposit() to use || instead of && to ensure the function reverts when either condition is violated, preventing excess tokens from being incorrectly handled.