slowfi - First Depositor Gets All Previous Rewards in `MlumStaking` contract

[sherlock-admin2]

slowfi

Medium

First Depositor Gets All Previous Rewards in MlumStaking contract

Summary

If rewards are minted to the MlumStaking contract before there is any open position, the first depositor gets all the rewards.

Vulnerability Detail

This issue just happens if there are rewards in the contract before any open position. After that opening two equal positions get a different amount of rewards, being the first depositor the one getting all.

Steps to reproduce:

1. Mint 100e6 to the MlumStaking contract.
2. Create a position for user0 with 1e18 on mlum and 0 days.
3. Create a position for user1 with 1e18 on mlum and 0 days.
4. Harvest user0 position and get the 100e6 rewards.
5. Harvest user1 position and get 0 rewards.

This happens because the first depositor gets the position.rewardDebt set to zero.

Impact

Improper rewards distribution.

Code Snippet

MlumStaking.sol#L656-L668

  function _updateBoostMultiplierInfoAndRewardDebt(StakingPosition storage position) internal {
      // keep the original lock multiplier and recompute current boostPoints multiplier
      uint256 newTotalMultiplier = position.lockMultiplier;
      if (newTotalMultiplier > _maxGlobalMultiplier) newTotalMultiplier = _maxGlobalMultiplier;

      position.totalMultiplier = newTotalMultiplier;
      uint256 amountWithMultiplier = position.amount * (newTotalMultiplier + 1e4) / 1e4;
      // update global supply
      _stakedSupplyWithMultiplier = _stakedSupplyWithMultiplier - position.amountWithMultiplier + amountWithMultiplier;
      position.amountWithMultiplier = amountWithMultiplier;

      position.rewardDebt = amountWithMultiplier * _accRewardsPerShare / PRECISION_FACTOR;
  }

Tool used

Manual Review

Recommendation

Reconfigure the distribution system considering if the rewards should be distributed between all positions at the moment of the first claim or if they shouldn't be distributed at all. Consider if the position.rewardDebt of the first depositor should be assigned differently in this particular case.

Duplicate of #116