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

0 stars 0 forks source link

No restriction on epocsDuration can lead to attacker gaining extra money #149

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

rishabh

Vulnerability details

Explanation

The contract does not follow 15 second rule for using block.timestamp ref epochDuration can be set to less than 15s this can lead to user taking more rewards than it is intented to

Let's assume:

epochDuration = 10 tokenPerEpoch = 1000 numberOfEpoch = 1000

Now let's say startTimestamp is 0 when creating the promotion so after 10s we can get our first claim [t=10s] after 20s we get our second claim [t=20s]

Now we execute the cancelPromotion function and with the help of powerful miner set the timestamp to be less than 15s from the actual timestamp meaning instead of 21s we set it to (21-15 = 6s)

Then, _getCurrentEpochId which is defined as (block.timestamp - _promotion.startTimestamp) / _promotion.epochDuration will return (6 - 0) / 10 which is 0

As EpochId is 0 we will get our full principal from `cancelPromotion` function

as we can see the attacker will be able to claim all his money in this case even after claiming his reward for older epochIds

Mitigation

The main problem is that epochDuration can be set to less than 15s and the miner can also manipulate timestamp by at max 15s.

Moderate epochDuration to be greater than 15s or Create a mapping for promotionId and epochId claimed and use it in cancelPromotion function

PierrickGT commented 2 years ago

This attack is not possible since the block timestamp increases monotonically and can't go back in time, even with the help of a miner. So the calculation (21-15 = 6s) doesn't work and at epoch id equal 0, the attacker would only be able to retrieve the full reward amount minus the first epoch reward amount, so not the full principal as stated in the issue. For this reason, I have disputed the issue.

dmvt commented 2 years ago

I agree with the sponsor here. Invalid report.