Users will lose funds forever when calling `unstake`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L290-L325

Vulnerability details

Impact

Users will lose funds when calling unstake. The unstake function will wrongly clear the stake amount and users will never get funds back.

Proof of Concept

2022/1/1: Alice stakes 1000 to TWELVE_MONTHS 2023/2/3: Alice stakes 10,000 to SIX_MONTHS, now the state is:

• 0 NONE
• 0 THREE_MONTHS
• 10,000 SIX_MONTHS (still staking)
• 1000 TWELVE_MONTHS (which can be unstaked)

Now, when Alice try to call unstake(1000), it will calculate totalVested = 0 + 0 + 0 + 1000 >= amount so it's able to unstake, then it will call _updateUserStakedAmounts. But in _updateUserStakedAmounts L306-L308, it will wrongly clear amount of SIX_MONTHS:

        if (amount > vestedSixMonths) {
          userstakedAmounts[user][Duration.SIX_MONTHS].amount = 0;
          userstakedAmounts[user][Duration.SIX_MONTHS].timestamp = 0;

Finally, the state will be:

• 0 NONE
• 0 THREE_MONTHS
• 0 SIX_MONTHS
• 0 TWELVE_MONTHS

Alice loses 10,000 tokens (in SIX_MONTHS amount) forever.

Tools Used

None

Recommended Mitigation Steps

It should check vested amount in every durations in _updateUserStakedAmounts:

  function _updateUserStakedAmounts(
    address user,
    uint256 amount,
    uint256 noVesting,
    uint256 vestedThreeMonths,
    uint256 vestedSixMonths,
    uint256 vestedTwelveMonths
  ) internal {
    if (amount > noVesting) {
      userstakedAmounts[user][Duration.NONE].amount = 0;
      userstakedAmounts[user][Duration.NONE].timestamp = 0;
      amount = amount - noVesting;
      if (amount > vestedThreeMonths) {
        if (vestedThreeMonths != 0) {
          userstakedAmounts[user][Duration.THREE_MONTHS].amount = 0;
          userstakedAmounts[user][Duration.THREE_MONTHS].timestamp = 0;
          amount = amount - vestedThreeMonths;
        }
        if (amount > vestedSixMonths) {
          if (vestedSixMonths != 0) {
            userstakedAmounts[user][Duration.SIX_MONTHS].amount = 0;
            userstakedAmounts[user][Duration.SIX_MONTHS].timestamp = 0;
            amount = amount - vestedSixMonths;
          }
          if (amount > vestedTwelveMonths) {
            revert();
          } else {
            userstakedAmounts[user][Duration.TWELVE_MONTHS].amount -= amount;
          }
        } else {
          userstakedAmounts[user][Duration.SIX_MONTHS].amount -= amount;
        }
      } else {
        userstakedAmounts[user][Duration.THREE_MONTHS].amount -= amount;
      }
    } else {
      userstakedAmounts[user][Duration.NONE].amount -= amount;
    }
  }

[nneverlander]

Duplicate. Fixed in https://github.com/infinitydotxyz/exchange-contracts-v2/commit/5f50384e2bc8f6f27914ec040ecb4a1464195678

[nneverlander]

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

[HardlyDifficult]

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