code-423n4 / 2021-12-pooltogether-findings

0 stars 0 forks source link

Reward can be cancel during a epoch #110

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

gzeon

Vulnerability details

Impact

When owner call cancelPromotion, the contract return rewards for all future epoch, including the current running epoch to the owner specified _to address. This is not a desired behavior as it allow people to create a huge reward to attract people to deposit, only to cancel the reward right before the epoch end, causing depositor to lose "expected" reward.

Proof of Concept

https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L130

Recommended Mitigation Steps

Subtract 1 epoch worth of token from the returned value, i.e. uint256 _remainingRewards = _getRemainingRewards(_promotion) - _promotion.tokensPerEpoch;

PierrickGT commented 2 years ago

We actually return the total rewards amount of the future epochs, without including the current one. https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L332 Let's say the total number of epochs is 12 and we are currently at epoch 7, we gonna subtract 12 - 7 = 5 epochs. So we send back tokensPerEpoch * 5 of total tokens and users can still claim rewards of epoch 7. For this reason, I've disputed the issue.

dmvt commented 2 years ago

Agree with sponsor here. Scenario described by warden is incorrect.