Summary

Currently the unstakeAll function enables a user to unstake from all the epochs at once, @dev This function allows users to unstake from all the epochs at once,, this might cause issues if newStaked is not well addressed.

Vulnerability Details

Currently the unstakeAll function enables a user to unstake from all the epochs at once, @dev This function allows users to unstake from all the epochs at once,, The functions achieves that because it iterates over all the epochs of the sender and updates accordingly based on this line below

Now that the function is getting all the activeDeposits of a user lets look at what happens when a user stakes,

Based on the code above, when a user stakes, the system first checks if the user is already staking in the current epoch. If the user has already staked in the current epoch, it updates the dr.staked value by adding the newly staked amount to the existing staked value. If the user has not yet staked in the current epoch, the system creates an activeDeposit for the user and logs the current epoch as the epoch they are staking in. After this, the newStaked value is updated with the staked amount.

This mechanism may cause issues. Consider a scenario where a user with stakes in other epochs stakes in the current epoch and then, during the same epoch, calls the unstakeAll function to unstake from all epochs. The problem here is that the newStaked value is not properly updated to account for the user's actions within the same epoch, which could lead to discrepancies.

Let's analyze why this could cause issues:

When a user stakes in an epoch, their staked amount is added to the newStaked value. This means that each time a new user stakes in the same epoch, the newStaked value is updated accordingly. However, when a user unstakes from all epochs using the unstakeAll function, the newStaked value is not properly adjusted to reflect the amount that has been unstaked. This leads to a situation where the newStaked value does not accurately represent the actual staked amount after the user has unstaked.

Specifically, even though the user's staked amount has been removed due to the unstake operation, the newStaked value still persists as if it includes the unstaked amount. As other users continue to stake in the same epoch, the newStaked value is further updated based on faulty data. This is problematic because the formula newStaked += _amount; is applied to a newStaked value that no longer accurately reflects the total staked amounts due to the prior unstake action.

The root of the issue is that the newStaked value is never properly adjusted in the unstakeAll function. If the system is intended to allow users to stake and immediately unstake within the same epoch, this discrepancy could lead to significant problems in how the newStaked value is managed and calculated.

Proof of concept(POC)

• User stakes 50 tokens in epoch 6. newStaked is updated to include the 50 tokens: newStaked += 50;

• User decides to call unstakeAll in epoch 6(the same epoch they staked).

• newStaked is not updated accordingly at the end to exclude the unstaked

• the staking process continuing in background and the falty newStaked versions are still being used when other users stake newStaked += _amount;

• After the epoch is done newStaked(now includes unstaked amounts) is added to the totalStaked totalStaked += newStaked;

• This results in newStaked not being properly adjusted

If multiple users are to do this now the totalStaked included a huge amount of already unstaked amounts totalStaked += newStaked;

since the newStaked values are not reduced to reflect actual values if users keep staking in the same epoch this means that the newStaked will show a different value which will be later added to the totalStaked amount.

Impact

Now that the totalStaked is faulty the rewards are not accurate users will get more

Tools Used

manual review

Recommendations

Consider adjusting the newStaked value to exclude the stake from current epoch if a user is to stake in it and call unstakeAll function