Submitted by EV_om, also found by 0xDING99YA and rvierdiiev
The ThrusterTreasure contract is designed to manage rounds of a lottery game, where participants can enter tickets and claim prizes based on random draws. The contract includes a variable maxPrizeCount which dictates the maximum number of prizes that can be set for any given round. This variable can be modified by the contract owner at any time through the setMaxPrizeCount(uint256 _maxPrizeCount) function:
ThrusterTreasure.sol#L139-L142
The issue arises when maxPrizeCount is decreased after prizes for a round have been set but before they have been claimed. Since the claimPrizesForRound(uint256 roundToClaim) function iterates over prize indices up to maxPrizeCount, reducing this count means that winners of prizes with indices higher than the new maxPrizeCount will be unable to claim their winnings:
ThrusterTreasure.sol#L102-L120
This could lead to a scenario where legitimate winners are denied their prizes due to a change in contract state that is unrelated to the rules of the game or their actions. Moreover, since calling claimPrizesForRound() clears the users entries for the round, reverting maxPrizeCount to its previous state does not allow them to claim the remaining tickets. This means they will effectively never be able to claim their prize.
To address this issue, implementing a checkpoint pattern for the maxPrizeCount variable is suggested. This method involves tracking changes to maxPrizeCount with checkpoints that record the value and the round number when the change occurs.
A possible implementation could look like this:
This change ensures that each rounds prize structure is fixed upon the rounds creation, preventing post-hoc alterations that could negatively impact participants. Note that this implementation still requires attention is paid to not calling setMaxPrizeCount() for a given round if prizes have already been set for higher indices.
jooleseth (Thruster) commented:
EV_om (warden) commented:
jooleseth (Thruster) confirmed
