First Flight #9: Soulmate

Summary

The Staking::deposit function allows the user who have LoveTokens to deposit amount of them and then to claim rewards based on the amount and the staked time. But if more people deposit large amount of tokens the total supply of stakingVault would be not enough to cover all rewards.

Vulnerability Details

The Staking::deposit function allows the user who have LoveTokens to deposit amount of them and then to claim rewards based on the amount and the staked time. The function makes check if the balance of stakingVault is not zero. But this check does not guarantee that there will be enough rewards for all future claims, especially if many users are staking large amounts of tokens. It only ensures that at the moment of deposit, the stakingVault is not completely empty.

Impact

Let's consider a scenario where the stakingVault has a non-zero balance that is not sufficient to cover the potential future rewards for all staked tokens, and a user tries to make a deposit. Assume the following:

The stakingVault contains 200 LoveToken to be distributed as rewards. There are 5 users who have each staked 50 LoveToken. According to the reward policy, users can claim 1 LoveToken per staked token per week. A new user, Charlie, wants to deposit 100 LoveToken into the Staking contract. Here's what happens:

• Charlie calls Staking::deposit function with amount equals to 100.

• The Staking contract checks the balance of the stakingVault:

• Since the stakingVault balance is 200 LoveToken, which is not zero, the deposit function proceeds without reverting.

• Charlie's deposit is successful, and his 100 LoveToken are transferred to the Staking contract. The userStakes mapping is updated to reflect Charlie's stake.

Now, if a week passes and all users, including Charlie, try to claim their rewards and no new users are deposited:

• The total staked tokens are now 350 LoveToken (5 existing users * 50 loveToken each + Charlie's 100 loveToken).

• The total rewards to be claimed by all users would be 350 LoveToken if each user claims for one week.
  But the stakingVault only has 200 LoveToken available for rewards.
  When users start claiming their rewards, the stakingVault will be depleted before all users have claimed their due rewards. The first two users to claim would receive their full rewards (100 LoveToken each), but the other users would receive nothing because the transferFrom function will revert due to arithmetic underflow or overflow.
  Also, if the stakingVault has some amount of tokens but not enough for rewards, these tokens will be left in the contract. It is better to have a dust collector like in the Airdrop::claim function.
  The Staking::deposit function does not account for the future obligations of the contract based on the current staking balance and the rewards policy.

Tools Used

Manual Review

Recommendations

The contract should implement a more sophisticated check that considers the total staked tokens and the duration of the staking to ensure that the stakingVault has enough funds to cover the rewards for all staked tokens, not just at the time of deposit but also for the foreseeable future.
Also, add a dust collector to the Staking::claimRewards function. In that way no amount would be left in the stakingVault.