`StakingServiceBase` is vulnerable to stepwise jump farming, which would reduce honest stakers yield

[hats-bug-reporter[bot]]

Github username: @NicolaMirchev Twitter username: nmirchev8 Submission hash (on-chain): 0xe076fc75a71e5a42be3fcafad3a6854142be426cc002da6f01015ea31611caed Severity: medium

Description: Description\

StakingServiceBase is the contract, which holds the main logic regarding stake accounting and reward distribution. Because of Convergence design choice to distribute rewards weekly (on cvgCycles), an interesting opportunity is opened, which is unfair, because expoiters has to stake for 1 block to receive the same reward as 1 week stakers. The main idea is that staker can deposit right before processStakersRewards(which will increment the cvgCycle) function is called. This is a problem, because expoiters are not obligated to have their funds locked, as they can transfer them and withdraw them once per week to claim rewards and during the week they their funds liquid and can use them to farm somewhere else.

Impact of the issue is far from neglectable, because main idea behind staking is to lock your funds, so they are useful somewhere else and that's why you earn yield/interest. Here this concept is broken and you can come and get one time per week

NOTE: Vulnerability is presen after expoiter has staked once and has waited 1 week (because of the following line)

Attack Scenario\

1. Bob and Eve stakes 1000e18 cvgCvx tokens to CvgCvxStakingPositionService for cvgCycle = 1
2. They have to wait for cvgCycle to be 3, so they can claim rewards for 1 and 2. This is because when they deposit _stakingHistoryByToken[tokenId].push(nextCycle) is set. In this case 2. So when they try to claim in second cycle, but haven't claimed other time, the following check would compare the above value (2) with the currentCycle = 2

       require(actualCycle > nextClaimableCvg, "ALL_CVG_CLAIMED_FOR_NOW");

3. But on cycle 3 they will manage to withdraw cvg rewards for cycle 1 and 2, and _nextClaims[tokenId].nextClaimableCvg will be set to 3.
4. After the claim in the beginning of cycle 3, Eve withdraw her 1000e18, swap them for something else and uses them whole week.
5. 1 block before cvgReward calls processStakersRewards with lets say amount of 1000e18, Eve again deposit with larger amount of 5000e18, because she will withdraw it with additional rewards in just a second.
6. Eve calls getAllClaimableCvgAmount and she claims more of the rewards, because only staked amount has weight, but no time staked.

               claimableAmount =
                   (tokenStaked * _cycleInfo[nextClaimableCvg].cvgRewardsAmount) /
                   _cycleInfo[nextClaimableCvg].totalStaked;
               /// @dev increments the total amount in CVG to mint to the user
               _totalAmount += claimableAmount;

7. Eve calls withdraw to get back her staked tokens, use them and do the same after one year.

Attachments

1. Proof of Concept (PoC) File PoC in comments

2. Revised Code File (Optional)

   • To mitigate it, you can implement time weighted factor, which is used when calculating rewards for stakers.

[PlamenTSV]

At step 5, when she restakes the larger amount, 5000 in your example, the cycle she would have done that at is 3, meaning her rewards become unclaimable until cycle 5. When you deposit at a cycle N, you increase the total staked amount for the next cycle N+1 and save the current cycle in your history struct. When claiming, you claim all unclaimed until <N+2. Another aspect is that the rewards received each cycle are based on the gauge's weights.

I could be missing something, will wait for your PoC.