First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

rarity in selectWinner will be same given fixed msg.sender, resulting in same type of token get drained

anjalit

Summary

selectWinner using hash of on-chain data for generating random number to calculate rarity generates same number everytime thus allowing same type of token get drained

Vulnerability Details

selectWinner calculates rarity using on-chain data block.difficulty and msg.sender . if we keep msg.sender same , rarity will be same in all the cases.

Doing some modifications in selectWinner such that it returns rarity (aka. randomNumber).

function selectWinner() external returns (uint256) {
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;
​
uint256 randomNumber = uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty)));
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);
​
return rarity;
}

here's a test case to check the randomness

  • suppose five players entered in the raffle and playerTwo being the one to call selectWinner

modifier playersEntered() {
address[] memory players = new address[](5);
players[0] = playerTwo;
players[1] = playerOne;
players[2] = playerThree;
players[3] = playerFive;
players[4] = playerFour;
puppyRaffle.enterRaffle{value: entranceFee * 5}(players);
_;
}
​
function test_SameRarityForAFixedSender() public playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
vm.prank(playerTwo);
​
(uint256 rarity) = puppyRaffle.selectWinner();
console.log(rarity);
}
​
/*
Logs:
31
*/

  • this time seven players entered the raffle and keeping sender same playerTwo again called selectWinner

modifier playersEntered() {
address[] memory players = new address[](7);
players[0] = playerTwo;
players[1] = playerOne;
players[2] = playerThree;
players[3] = playerFive;
players[4] = playerFour;
players[5] = playerSix;
players[6] = playerSeven;
puppyRaffle.enterRaffle{value: entranceFee * 7}(players);
_;
}
​
function test_SameRarityForAFixedSender() public playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
vm.prank(playerTwo);
​
(uint256 rarity) = puppyRaffle.selectWinner();
console.log(rarity);
}
​
/*
Logs:
31
*/

clearly visible that rarity was same in both cases even tho different number of players entered in the raffle and the reason behind is random number generated being same.

Impact

Due to source of randomness not working properly all the tokens of similar rarity, in the upper case of COMMON_RARITY (as 31<=70) will get drained which can cause imbalance between different type of tokens plus it doesn't stand out on the behavior selectWinner was supposed to have

Tools Used

Foundry

Recommended Mitigation

Consider to not use on-chain data as the source of randomness to calculate rarity.

One way of achieving this is to use chainlink VRFs as source to generte random number and calculate rarity.

Updates

Lead Judging Commences

Hamiltonite Lead Judge about 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.

Give us feedback!