Liquid Staking

Summary

The deposit function in StakingPool.sol

• When deposit is called with _amount = 0, the function still calls _depositLiquidity(_data), which can deposit tokens into strategies.

• However, the totalStaked variable is only updated when _amount > 0, meaning that any changes made by _depositLiquidity are not accounted for in totalStaked.

• This can cause totalStaked to be lower than the actual total amount of staked tokens across the pool and its strategies.

StakingPool.sol, specifically in this block: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L120-L127

Connects to:

• The _depositLiquidity function, which deposits tokens into strategies based on the provided _data.

• The totalStaked variable, which keeps track of the total staked amount in the pool.

• The _mint function, which mints liquid staking tokens to the user when they deposit.

Vulnerability Details

The deposit function in takingPool contract does not update the totalStaked variable when the _amount parameter is zero.

Because the function still calls _depositLiquidity(_data) even when _amount is zero, which can deposit tokens into strategies. However, totalStaked is only updated when _amount is greater than zero, meaning that any changes made by _depositLiquidity are not accounted for in totalStaked. This can cause totalStaked to be lower than the actual total amount of staked tokens across the pool and its strategies.

The deposit function: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L120-L132

To reproduce (Proof of Concept):

1. Have some strategies set up in the StakingPool contract.

2. Call deposit with _amount = 0 and _data that deposits tokens into one or more strategies.

3. Observe that totalStaked remains unchanged, even though tokens were deposited into the strategies.

4. Call withdraw with an amount less than or equal to the actual total staked amount (including the tokens deposited in step 2).

5. Observe that totalStaked is reduced by the withdrawn amount, leading to an inconsistency between totalStaked and the real total staked amount.

Impact

This bug can affect users in the following ways:

• If totalStaked is lower than the actual total staked amount, it may prevent users from withdrawing their full balances, as the contract relies on totalStaked to determine the available withdrawal amount.

• Inconsistencies in totalStaked can lead to incorrect reward calculations and distributions, as rewards are often based on the proportion of a user's stake relative to the total staked amount.

• If the bug is exploited repeatedly, it can gradually drain the pool's funds, as totalStaked can be manipulated to be lower than the real total staked amount, allowing more tokens to be withdrawn than should be possible.

Tools Used

Vs Code

Recommendations

By capturing the initial value of totalStaked, depositing liquidity, and then updating totalStaked based on the changes made by _depositLiquidity and the deposited _amount, the function ensures that totalStaked accurately reflects the total staked amount across the pool and its strategies.