Handle

certora

Vulnerability details

claimRewards gets epochs ids as uint256[]. However, it should be uint8[]. If a user provides an epoch Id that's larger than 256, _isClaimedEpoch will return false:

function _isClaimedEpoch(uint256 _userClaimedEpochs, uint256 _epochId)
internal
pure
returns (bool)
{
    return (_userClaimedEpochs >> _epochId) &  uint256(1) ==  1;
}

This allows claiming rewards for ids greater than 256. In addition, in _calculateRewardAmount, the epoch start time and end time are calculated:

uint256 _epochStartTimestamp = _promotion.startTimestamp + (_epochDuration * _epochId);
uint256 _epochEndTimestamp = _epochStartTimestamp + _epochDuration;

and then are casted to uint64 for the rest of the function. if it's greater than 2**64, it will be truncated. It allows anyone to provide epoch id such that the epoch start time will be truncated to a valid start time, because they can provide an epoch id as large as they want because _isClaimedEpoch will return false for numbers greater than 256.

After that, they can claim rewards for this valid start and end times, with an invalid epoch id, and they can do it as many times as they want.

Impact

A malicious user can claim rewards on the same epoch multiple times and drain the promotion.

Proof of Concept

Let's assume that the first epoch started on timestamp = 100 and ended on timestamp = 110. and epoch duration is 10. I can call claimRewards with _epochIds = [2**64 / 10] it'll pass the check :

require(
    !_isClaimedEpoch(_userClaimedEpochs, _epochId),
    "TwabRewards/rewards-already-claimed"
);

because it's greater than 256.

uint256 _epochStartTimestamp = _promotion.startTimestamp + (_epochDuration * _epochId);

so it'll be equal to 100 + 10 * 2**64 / 10 = 100 +2**64. then truncated to uint64(100 +2**64) = 100. It is a valid start time, and I can claim rewards infinite times.

Tools Used

manual analysis

Recommended Mitigation Steps

change claimRewards signature to:

function claimRewards(
    address _user,
    uint256 _promotionId,
    uint8[] calldata _epochIds                                                                                                                                                                                             
)

(uint8[] instead uint256[])

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