TwabRewards: fee on transfer token as promotion token can make claimRewards() & cancelPromotion() fail

[code423n4]

Handle

GiveMeTestEther

Vulnerability details

Vulnerability details

Impact

If the rewards creator sets a token that applies transfer fees for the promotion token, users won't be able to claim (some of) their rewards because the claimRewards() uses _calculateRewardAmount() that assumes for each epoch there are "_promotion.tokensPerEpoch" amount of the promotion token available. Then the safeTranfser is called with an amount higher than the contracts holds and fails.

Also the cancelPromotion() will fail, because _getRemainingRewards() also assumes there are _promotion.tokensPerEpoch for each epoch available and the transfer will fail. (this alone could be seen as separate medium bug, because this would make the cancelPromotion() unusable)

those funds that can't be claimed by an user because of the failing transfer, will be locked forever in this contract, also the creator won't be able to get those token by calling cancelPromotion() because this will fail also, so I would classify this combination as high risk (each alone I would classify as medium).

Proof of Concept

Assumptions:

• 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
• We 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.

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

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

Tools Used

Manual Analysis

Recommended Mitigation Steps

• whitelist tokens
• or some complicated bookkeeping that will become very gas inefficient

[itsmetechjay]

Withdrawn by warden. Per GiveMeTestEther, "could you please delete this issue?"