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

There is No Randomness in selectWinner()

Summary

Randomness is not truly random and user can use this to choose themselves as winner and get a legendary puppy.

Vulnerability Details

Winner and rarity is calculated as:

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

The variables under consideration are not completely randomized and their outcomes can be predicted. These values are accessible to any miner. Any other transaction within the same mined block has the ability to read these quantities. The situation can further deteriorate as these values are susceptible to manipulation if the attacker also happens to be a miner.
Hence it is possible for a miner to manipulate timestamp and difficulty in order to choose themselves as winner and get the rarest puppy. Let's not forget that this function is external and anyone can call it.

Impact

Main functionality of the protocol (raffle) is broken. It is hackable by miners and is not provide valid randomness. Impact is high and likelihood is medium, hence I consider this as high.

Tools Used

Manual Review

Recommendations

Find a better way to create randomness. Chainlink VRF is a good example.

Updates

Lead Judging Commences

Hamiltonite Lead Judge almost 2 years 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.