Incorrect accounting results in loss of user stake

[code423n4]

Lines of code

InfinityStaker.sol#L116-L131 InfinityStaker.sol#L290-L325

Vulnerability details

Impact

In InfinityStaker.sol, when calling the unstake function _updateUserStakedAmounts is called to readjust the user staked amounts. If a user stakes an amount in a duration and has an already vested amont in a higher duration, the user might lose the not yet vested stake in the lower duration.

This is due to the _updateUserStakedAmounts function setting the userStakedAmount on all durations to 0 if its vesting amount is 0 without checking if the duration has a non-zero staked amount.

Proof of Concept

1. A user has 1000 tokens staked in the TWELVE_MONTH duration which can be vested.

2. Before unstaking the vested TWELVE_MONTH stake, the user then stakes an additional 2000 tokens into the THREE_MONTH duration.

3. The user then calls unstake to unstake the 1000 tokens in the TWELVE_MONTH duration. (Before the THREE_MONTH stake can be vested)

4. The _updateUserStakedAmounts will then check if the unstake amount is more than the amount which can be vested for each duration.

if (amount > vestedThreeMonths) {
        userstakedAmounts[user][Duration.THREE_MONTHS].amount = 0;
        userstakedAmounts[user][Duration.THREE_MONTHS].timestamp = 0;
        amount = amount - vestedThreeMonths;

5. Since vestedThreeMonths is 0, the THREE_MONTH stake amount is set to 0 and eventually the TWELVE_MONTH stake is decreased by 1000.

6. The user receives 1000 tokens and loses the 2000 tokens staked in THREE_MONTHS.

Recommended Mitigation Steps

The userStakedAmount should only decrease if the vested amount > 0. Consider adding an additional check for each if statement in the _updateUserStakedAmounts to check if the vested amount for that duration is more than 0 and to only reset the duration's stake if this is true.

[nneverlander]

Duplicate

[HardlyDifficult]

Dupe https://github.com/code-423n4/2022-06-infinity-findings/issues/50