If the promotion token applies transfer fees, the total amount to claim will be less than "_tokensPerEpoch * _numberOfEpochs" ( bcs a part of this amount is the fee => (funds + fee), but only the "funds" can be withdrawn) but the calculation in "_calculateRewardAmount" is based on "_promotion.tokensPerEpoch" . This implies that if the rewards of a user sum up to the "remaining funds +fee" of the promotions tokens that the TwabRewards contract holds (the user is the last one claiming the rewards), the user won't be able to withdraw rewards for at least one epoch, bcs the safeTranfser() is called with an amount (includes the fee) higher than the contract holds.
This case happens most likely after the promotion has ended.
There is no way to withdraw those funds and they are locked forever in this contract (loss of funds).
Proof of Concept
Assumptions for a simple example:
There is only one promotion with this promotion token (if there are multiple)
There is only one user that holds all the prize pool tickets, so only this user can claim any rewards.
User holds the same amount of tickets over a period of time such that TWAB becomes a constant function
The rewards creator sets a promotion token with a fee
Only have one epoch (_numberOfEpochs = 1)
In the "createPromotion()" the TwabRewards contract will receive "_tokensPerEpoch - fee" and not "_tokensPerEpoch".
If the user wants to claim the rewards after _epochEndTimestamp has passed the "_calculateRewardAmount()" will return _promotion.tokensPerEpoch. Therefore the "_rewardsAmount" will be equal to "_promotion.tokensPerEpoch.".
The "claimRewards()" will try to "_promotion.token.safeTransfer(_user, _rewardsAmount);" but the contracts has only "_tokensPerEpoch - fee" of the promotion tokens and the safeTransfer will fail. User won't be ever able to claim the rewards.
PierrickGT
commented
2 years ago
Handle
GiveMeTestEther
Vulnerability details
Impact
If the promotion token applies transfer fees, the total amount to claim will be less than "_tokensPerEpoch * _numberOfEpochs" ( bcs a part of this amount is the fee => (funds + fee), but only the "funds" can be withdrawn) but the calculation in "_calculateRewardAmount" is based on "_promotion.tokensPerEpoch" . This implies that if the rewards of a user sum up to the "remaining funds +fee" of the promotions tokens that the TwabRewards contract holds (the user is the last one claiming the rewards), the user won't be able to withdraw rewards for at least one epoch, bcs the safeTranfser() is called with an amount (includes the fee) higher than the contract holds.
This case happens most likely after the promotion has ended.
There is no way to withdraw those funds and they are locked forever in this contract (loss of funds).
Proof of Concept
Assumptions for a simple example:
In the "createPromotion()" the TwabRewards contract will receive "_tokensPerEpoch - fee" and not "_tokensPerEpoch". If the user wants to claim the rewards after _epochEndTimestamp has passed the "_calculateRewardAmount()" will return _promotion.tokensPerEpoch. Therefore the "_rewardsAmount" will be equal to "_promotion.tokensPerEpoch.".
The "claimRewards()" will try to "_promotion.token.safeTransfer(_user, _rewardsAmount);" but the contracts has only "_tokensPerEpoch - fee" of the promotion tokens and the safeTransfer will fail. User won't be ever able to claim the rewards.
https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L162 https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L289
Tools Used
Manual Analysis
Recommended Mitigation Steps