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

selectWinner() function's logic, using a know hash technique of on-chain data generates non-random numbers where needed

PoC for weak PRNG Vulnerability

Summary

The PuppyRaffle::selectWinner() function's logic, using a hash of on-chain data to generate:

  • the random number used to select the winner

  • the random number used to select the rarity of the NFT

is weak and can be used to predict the outcomes.

Vulnerability Details

The function uses a widely known hashing algorithm (keccak256) and available to all data to generate numbers that will be used to pick the raffler winner withing the players array aswell as the rarity of the NFT that is to be minted to him.

A hacker could anticipate and make sure to enter the raffle at the right time, in order for his slot i in the players array to be equal to the number that he could generate using his address, a chosen block.timestamp and the current or anticipated block.difficulty. He then would make sure he is the winner of the raffle with and NFT of highest rarity.

Impact

HIGH IMPACT:

If a hacker attacks the contract in this way, he would have the money directly sent to him since he would be the wiiner (even though he would have cheated).

LOW LIKELYHOOD:

Calculating and coordinating to reach this objective is quite technical: it's not a simple hack.

Tools Used

  • VScode

  • Slither

Recommendations

  1. Remove the weak "not so random" generating code for winner selection and rarity selection:

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];
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
totalFees = totalFees + uint64(fee);
uint256 tokenId = totalSupply();
- // We use a different RNG calculate from the winnerIndex to determine rarity
- uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100;
if (rarity <= COMMON_RARITY) {
tokenIdToRarity[tokenId] = COMMON_RARITY;
} else if (rarity <= COMMON_RARITY + RARE_RARITY) {
tokenIdToRarity[tokenId] = RARE_RARITY;
} else {
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}
delete players;
raffleStartTime = block.timestamp;
- previousWinner = winner;
- (bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
- _safeMint(winner, tokenId);
}
  1. Implement the use of an oracle to make sure that the numbers used to select the winner and the NFT's rarity are randomly generated off chain.

    See: https://docs.chain.link/vrf

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.