Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

Anyone could be the winner in selectWinner() due to predictable random number

Summary

The pseudo random number, winnerIndex, in selectWinner() is predictable such that bad actors could always be the winner and get the prize.

Vulnerability Details

function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];

Since winnerIndex is derived from msg.sender, block.timestamp, and block.difficulty, in any given block, we can derive the winnerIndex in a smart contract before invoking selectWinner(). In addition, we can generate many "msg.sender" (e.g., with create2) to let the winnerIndex points to a specific player. Therefore, whenever we have players.length >= 3, we can intentionally be the 4th player with a "msg.sender" deriving a winnerIndex==3 and walk away with all prize.

Impact

Anyone implementing the above logic could always be the winner

Tools Used

Manual review

Recommendations

Use chainlink VRF to choose the winner

Updates

Lead Judging Commences

Hamiltonite Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

weak-randomness

Root cause: bad RNG Impact: manipulate winner

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.