First Flight #2: Puppy Raffle

Summary

In the PuppyRaffle::selectWinner() function, it's advisable to replace the condition >= with >. The raffle officially concludes when block.timestamp exceeds raffleStartTime + raffleDuration. Since block timestamps don't consistently occur every second, there's a risk that block.timestamp might be equal to raffleStartTime + raffleDuration while the raffle is still technically active, especially when using >=. To ensure the raffle is truly over, it's recommended to use the condition > raffleStartTime + raffleDuration.

Vulnerability Details

Technically speaking, the raffle has officially ended, i.e. not active anymore, once time > raffleStartTime + raffleDuration.
And since a new block.timestamp doesn't consistently happen every single moment or second, there is the risk of current block.timestamp being equal to raffleStartTime + raffleDuration while the raffle is technically still active, for the case where we use >=:

But the raffle is not over at == raffleStartTime + raffleDuration, it is only technically over at > raffleStartTime + raffleDuration.

All in all, it would potentially make it possible to end the raffle and select the winner in the same block, which is unlikely to be the intention of the project. Generally we would want the winner to be selected at least in the next block after the raffle ended, to be sure we dont invite any related potential edge cases that way.

Impact

Edge case where winner is selected at the same time the raffle is technically still active, as well as selecting winner in same block as when raffle ends.

Deemed low for now but I suspect it could be a medium risk issue, especially if we start involving miners/mev bots who intentionally target this "vulnerability".

Tools Used

VSC.

Recommendations