First Flight #2: Puppy Raffle

Summary

The winnerIndex calculation is not truely random and can be exploited by a player so that they always win.

Vulnerability Details

The variables used in the 'keccak256()' hash function are all known values. selectWinner() can be called by any address, so msg.sender is known (and set), block.timestamp and block.difficulty can be determined also. The players array is public so its length is also known and can be modified by the exploiter by entering new players. As the index is modulo players.length, by being able to change the length of the players array, the exploiter can ensure the winnerIndex is for an address they own.

Impact

Breaks the 'lottery' aspect, an exploiter can win every time. High Severity as the contracts purpose/functionality is fully broken (fair lottery system).

Tools Used

Recommendations

Implement the 'Commit/Reveal' Scheme for random number generation, or even better use Chainlink VRF, for random number generation.