First Flight #2: Puppy Raffle

Summary

HIGH-3: In the "selectWinner" function, the calculation of the winnerIndex is not truly randomly generated.

uint256 winnerIndex = uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;

Vulnerability Details

The winnerIndex creates a keccak256 hash of three variables. msg.sender (the address of the caller of the function), block.timestamp (the time when the tx block was created), and block.difficulty. All these variables can be retrieved from the chain or are known to the sender. The hashing algorithm is also a publically available program that can help malicious users help in determining when to call the function to be a winner. They can retrieve their address index using the public function "getActivePlayerIndex" and similarly they may check on the length of the "players" array as it is declared as public variable. Such calculation threatens the randomness of selecting the winner.

Impact

Potential to bend the contract to the attackers will, making him win the raffles unfairly by using the vulnerability to wait for the conditions to favor one of its addresses in the "players" array.

Tools Used

Static analysis, local testing

Recommendations

Best practice would be using oracles - off-chain system for providing random data with low/no risk of being manipulated. Alternativelly, including the block nonce in the hashing process might help mitigating the risk, because nonce is being as close on the chain as it gets.