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

Weak randomness in the `selectWinner()` function results in anyone being able to pick the winner

Summary

The function PuppyRaffle::selectWinner uses block.timestamp and block.difficulty to select random index of the winner. But the function selectWinner() is external and block.timestamp and block.difficulty can be manipulated and a specific winner can be chosen by anyone.

Vulnerability Details

In Ethereum, the block.timestamp and block.difficulty are global variables that are determined by the miner who mines the block. This means that miners have some degree of control over these values, which can be exploited to manipulate the outcome of functions that rely on them for randomness.

Miners have the ability to adjust the block.timestamp of the blocks they mine by up to roughly 900 seconds (15 minutes) into the future. This is because the Ethereum protocol only requires that each block's timestamp is greater than its parent's.

If a miner knows how the block.timestamp will be used to determine the winner of the raffle, they could adjust the timestamp of the block they're mining to influence the outcome. If the miner is also a player in the raffle, they could potentially adjust the timestamp to ensure they win.

The block.difficulty represents the difficulty level of the mathematical problem that must be solved to mine a block. Miners have some control over this value because they can vary the amount of computational power they use to mine blocks.

If a miner knows how the block.difficulty will be used in the keccak256 function to determine the winner of the raffle, they could vary their computational power to influence the block.difficulty and hence the outcome of the raffle.

This ability to manipulate block.timestamp and block.difficulty is why it's generally recommended not to use these values as sources of randomness in smart contracts.

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);
}

Impact

The function selectWinner() uses block.timestamp and block.difficulty to generate a pseudo-random number for selecting the winner. This is considered weak because miners have some control over the block timestamp and difficulty, which could potentially be exploited to manipulate the outcome of the raffle.
The block.timestamp influences the keccak256 function, and subsequently the selectWinner() function, by being part of the seed that generates the pseudo-random number. In the selectWinner() function, the seed for the keccak256 function is likely created by packing msg.sender, block.timestamp, and block.difficulty together using abi.encodePacked.

To demonstrate the weak randomness in the selectWinner() function, we can write a test case that manipulates the block.timestamp and block.number using the vm.warp and vm.roll functions in Foundry.
The vm.warp and vm.roll functions in Foundry give control over the simulated blockchain environment in the tests. These functions don't directly influence the calculation of the winner in the selectWinner() function, but they do allow to manipulate the values that are used in that calculation. The vm.warp function allows to manipulate the block.timestamp value in the test. The block.timestamp value is one of the inputs to the keccak256 function in selectWinner(), which generates a pseudo-random number to determine the winner.
The vm.roll function allows you to advance the block.number in the tests. Although block.number is not directly used in the selectWinner() function, it can indirectly influence the outcome because Ethereum's block.difficulty value can change from block to block.
Add the following test case in file PuppyRaffleTest.sol:

function testWinner3() public playersEntered {
vm.warp(block.timestamp + duration + 2);
vm.roll(block.number + 1);
// Call the selectWinner function
puppyRaffle.selectWinner();
address winner1 = puppyRaffle.previousWinner();
address[] memory players = new address[](4);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
players[3] = playerFour;
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
// Warp the block timestamp and roll the block number to the same values as before
vm.warp(block.timestamp + duration + 2);
vm.roll(block.number + 1);
// Call the selectWinner function again
puppyRaffle.selectWinner();
address winner2 = puppyRaffle.previousWinner();
// Assert that the winner is the same player as before
assertEq(winner1, winner2);
}

The test function testWinner3() shows that by certain parameters of vm.warp and vm.roll the output will be the same. In this test case we want the winner to be playerThree. And after executing the selectWinner() function twice according to certain parameters, the winner is the same (playerThree).
Also, the function selectWinner() is external with no access control and anyone can call the function and manipulate the winner.

Tools Used

VS Code, Foundry

Recommendations

You might consider using a dedicated randomness oracle such as Chainlink VRF.
https://blog.chain.link/random-number-generation-solidity/

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.