Submitted by johnnycash, also found by certora, cmichel, gpersoon, gzeon, harleythedog, kemmio, kenzo, sirhashalot, and 0x421f
An attacker can claim its reward 256 * epochDuration seconds after the timestamp at which the promotion started. The vulnerability allows him to claim a reward several times to retrieve all the tokens associated to the promotion.
claimRewards() claim rewards for a given promotion and epoch. In order to prevent a user from claiming a reward multiple times, the mapping _claimedEpochs keeps track of claimed rewards per user:
(The comment is wrong, epochs are packed into a uint256 which allows 256 epochs to be stored).
_epochIds is an array of uint256. For each _epochId in this array, claimRewards() checks that the reward associated to this _epochId isnt already claimed thanks to
_isClaimedEpoch(). _isClaimedEpoch() checks that the bit _epochId of _claimedEpochs is unset:
However, if _epochId is greater than 255, _isClaimedEpoch() always returns false. It allows an attacker to claim a reward several times.
_calculateRewardAmount() just makes use of _epochId to tell whether the promotion is over.
The following test should result in a reverted transaction, however the transaction succeeds.
A possible fix could be to change the type of _epochId to uint8 in:
and change the type of _epochIds to uint8[] in claimRewards().
PierrickGT (PoolTogether) confirmed
