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

`PuppyRaffle::selectWinner` winnerIndex is not truly random and can be rigged

maroutis

Summary

The PuppyRaffle::selectWinner winnerIndex is predictable in advance to know exactly which index will win. An player can know at each moment who the winner of the lottery is. Plus any miner that has control over block.timestamp and/or block.difficulty can manipulate it to his benefit.
The rarity calculation is also impacted but has less impact.

Vulnerability Details

​
/// @notice this function will select a winner and mint a puppy
/// @notice there must be at least 4 players, and the duration has occurred
/// @notice the previous winner is stored in the previousWinner variable
@> /// @dev we use a hash of on-chain data to generate the random numbers
/// @dev we reset the active players array after the winner is selected
/// @dev we send 80% of the funds to the winner, the other 20% goes to the feeAddress
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;

The comments suggest that the winner is randomly chosen from the array of players. But that is not true because :

  1. The value of uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length can be known in advance by anyone. Consequently, a player can add multiple wallet addresses of their own to the players array. Then he will calculate the winning index for each addition to see if any of their addresses would be chosen as the winner. Finally, he will execute the PuppyRaffle::selectWinner function when they confirm one of their addresses would be chosen.

  2. The value uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length can be manipulated by miners in favor of a particular winner. Since they have control over block parameters like block.timestamp and block.difficulty, a miner can iterate through different block hashes until the outcome matches a desired winner.

The rarity calculation can also be manipulated:

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

Impact

The lottery can be rigged to favor a particular winner. The core functionality is thus compromised. Users will not use the protocol because of lack of trust.

Proof of concept:

There are many ways to rig this protocol. Here is one of them, the Miner can just change block.difficulty to favor a certain player right before selecting a winner. To execute this test : forge test --mt testMinerChangesBlockDiffiultyToFavorPlayerOne -vv

function testMinerChangesBlockDiffiultyToFavorPlayerOne() public playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
address miner = makeAddr("Miner");
​
uint256 length = 4;
​
vm.startPrank(miner);
​
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(miner, block.timestamp, block.difficulty))) % length;
console.log(winnerIndex);
// Initially, WinnerIndex = 3
​
​
// Miner will now manipulate the block.difficulty so that the first player will be the winner
vm.prevrandao(bytes32(uint256(19)));
​
uint256 RiggedWinnerIndex =
uint256(keccak256(abi.encodePacked(miner, block.timestamp, block.difficulty))) % length;
console.log(RiggedWinnerIndex);
// RiggedWinnerIndex = 1
​
puppyRaffle.selectWinner();
// Check that the first player is the winner = profit
assertEq(puppyRaffle.previousWinner(), playerOne);
​
vm.stopPrank();
}

Tools Used

  • foundry

Recommendations

  • Use a real fair randomizer to chose the winner. For example, the Chainlink VRF is a well proven fair and audited randomizer. Miners will not be able to manipulate it.

  • Add an onlyOwner modifier to the function PuppyRaffle::selectWinner. Since Users will not know the exact time of the winner selection. They will be discouraged from adding many of their own address as players right before the execution of the function, to increase their chances.

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!