search

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

0 stars 0 forks source link

Suggestion : To add explicit check for no of epochs to be 255 in createPromotion and extendPromotion #147

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0x421f

Vulnerability details

So we are using uint256 to store 1/0 if that epoch is being claimed or not uint256(1) is our base, so 255 is max limit we have

now if someone creates a promotion with >255 epochs in epochs > 255 , it will revert with "TypeError : Invalid Rational Number" (Copied from remix. I suppose its due to overflow)

Hence there is no chance of cycle, or someone claiming funds beforehand or on repeat. No funds are at risk, but its bad UX for both user and creator now only option creator would have is cancelPromotion and create new

Hence imo We should restrict it on top only at time of creation and extension ie. noOfEpochs <= 255

Thanks

PierrickGT commented 2 years ago

Will be fixed with this issue: https://github.com/code-423n4/2021-12-pooltogether-findings/issues/3