If the promotion token applies transfer fees, the total amount the contract holds will be less than "_tokensPerEpoch * _numberOfEpochs"( bcs a part of this amount is the fee => (funds + fee), but only the "funds" can be withdrawn). If after each epoch all the user claim their rewards directly the cancelPromotion() will fail bcs the amount returned from _getRemainingRewards() will also include the fee and is equal to "remaining funds + fee". Therefore we try to withdraw an amount higher than this contracts holds and revert.
If not all users have claimed their rewards, we most likely can execute "cancelPromotion()" but at least one user won't be able to withdraw the rewards for at least one epoch, bcs the "fee" amount is now missing from the claimable rewards and there will be a safeTransfer (last user claiming the rewards) with an amount higher than than the contract holds.
The rewards for this user from this unclaimable epoch will be lost forever (lost of funds).
Also the cancelPromotion() will fail in 0 epoch, bcs in this case we try to withdraw "funds + fee".
PierrickGT
commented
2 years ago
Handle
GiveMeTestEther
Vulnerability details
Impact
If the promotion token applies transfer fees, the total amount the contract holds will be less than "_tokensPerEpoch * _numberOfEpochs"( bcs a part of this amount is the fee => (funds + fee), but only the "funds" can be withdrawn). If after each epoch all the user claim their rewards directly the cancelPromotion() will fail bcs the amount returned from _getRemainingRewards() will also include the fee and is equal to "remaining funds + fee". Therefore we try to withdraw an amount higher than this contracts holds and revert.
If not all users have claimed their rewards, we most likely can execute "cancelPromotion()" but at least one user won't be able to withdraw the rewards for at least one epoch, bcs the "fee" amount is now missing from the claimable rewards and there will be a safeTransfer (last user claiming the rewards) with an amount higher than than the contract holds. The rewards for this user from this unclaimable epoch will be lost forever (lost of funds).
Also the cancelPromotion() will fail in 0 epoch, bcs in this case we try to withdraw "funds + fee".
Proof of Concept
https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L289
https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L331
Tools Used
Manual Analysis
Recommended Mitigation Steps