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

current randmoness is predictable, and can be manipulated

0xtheblackpanther

Summary

The contract relies on on-chain data for randomness, which can be predictable and subject to manipulation, raising concerns about the fairness of winner selection

Vulnerability Details

The contract employs the keccak256 function for generating randomness to select a winner, which, in an on-chain context, can be predictable and potentially manipulated by malicious actors. This predictability undermines the integrity of the selection process, potentially allowing unfair advantages or gaming of the system

Impact

The predictable and manipulable randomness mechanism in the contract can lead to several adverse consequences. It may allow malicious users to influence the selection of winners, thereby compromising the fairness and integrity of the raffle. This impact could result in a loss of trust and reputation for the contract and its operators, as well as potential financial losses for participants.

Tools Used

  • manual review & foundry

Recommendations

Consider implementing a more secure source of randomness like external oracles or Chainlink VRF for improved raffle fairness and security.

You can implement Chainlink VRF in the selectWinner function to replace the current randomness generation. Here's an example of how you can modify the selectWinner function to use Chainlink VRF for generating random numbers:

Steps:

1 - Import VRFConsumerBase import "@chainlink/contracts/src/v0.8/VRFConsumerBase.sol";
2 - Import VRFConsumerBase to PuppyRaffle contract

-contract PuppyRaffle is ERC721, Ownable {
+contract PuppyRaffle is ERC721, Ownable, VRFConsumerBase { // Import VRFConsumerBase

3 - Declare these vars and add the below to constructor

uint256 public randomResult; // Store the random number
​
// Constructor with Chainlink VRF configuration
constructor(
uint256 _entranceFee,
address _feeAddress,
uint256 _raffleDuration,
address _vrfCoordinator,
address _link,
bytes32 _keyHash,
uint256 _fee
) ERC721("Puppy Raffle", "PR") VRFConsumerBase(_vrfCoordinator, _link) {
entranceFee = _entranceFee;
feeAddress = _feeAddress;
raffleDuration = _raffleDuration;
raffleStartTime = block.timestamp;
​
// ... Other constructor code ...
​
keyHash = _keyHash; // Chainlink VRF key hash
fee = _fee; // Chainlink VRF fee
}

4 - And now add the below function for fair randomness

function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
​
// Request random number from Chainlink VRF
requestRandomNumber();
​
// The actual winner selection will occur in fulfillRandomness function
}
​
// Chainlink VRF callback to handle random number
function fulfillRandomness(bytes32 requestId, uint256 randomness) internal override {
require(msg.sender == vrfCoordinator, "Fulfillment only allowed by Coordinator");
randomResult = randomness;
​
// Use the randomResult for winner selection
uint256 winnerIndex = randomResult % 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);
}
​
// ... Other contract functions ...
​
// Function to request a random number from Chainlink VRF
function requestRandomNumber() private {
require(LINK.balanceOf(address(this)) >= fee, "Not enough LINK to fulfill request");
bytes32 requestId = requestRandomness(keyHash, fee);
// You can store the requestId for reference if needed
}
​

Make sure to add test LINK

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.