Root + Impact

Description

PuppyRaffle.selectWinner() in src/PuppyRaffle.sol generates both the winning index and the puppy rarity from on-chain values that are visible to everyone before the transaction is mined, and that the caller and the block producer can influence. The raffle is therefore not random in any meaningful sense.

The vulnerable code is:

and, for rarity:

Every input to these hashes is either chosen by the caller or public before the call. msg.sender is the address of whoever calls selectWinner(), and anyone may call it, so the caller knows their own input exactly. block.timestamp is the timestamp of the block the transaction lands in, which the caller can estimate closely and which a block producer can adjust within a small range. block.difficulty is also public, and on proof-of-stake networks it reflects the previous randomness value, which is likewise visible in advance. Hashing public inputs does not make the result unpredictable: anyone can compute the same keccak256 hash off-chain, with the same inputs, and get the same winner index and rarity before deciding whether to call the function.

There is no commit-and-reveal step, no verifiable random function, and no delay between the inputs becoming known and the winner being chosen. The contract comments even say that a hash of on-chain data is used to generate the random numbers, which confirms that this is the intended mechanism, not a hidden fallback.

Risk

Likelihood: Medium - exploiting this needs some preparation, but the preparation is ordinary off-chain calculation. An attacker can simulate the outcome for a coming block, wait for a block in which the hash selects them, and only then call selectWinner(). A block producer has even more direct influence over the timestamp and can order transactions to suit themselves.
Impact: High - the winner of the raffle, the 80% prize pool, and the minted puppy NFT can all be steered to an attacker instead of being drawn fairly. The rarity is affected in the same way, so an attacker can also aim for a legendary puppy, which is meant to be the rarest outcome. Honest players pay the entrance fee for a fair chance they do not actually get, and the raffle's whole purpose is undermined.

Proof of Concept

Manual trace - no tests were run; this is a step-by-step reading of the code, with a worked example.

Worked example: suppose four players have entered, so players.length is 4, and the raffle duration has passed.

1. An attacker who is one of the four players reads the current block timestamp and difficulty from the chain, and knows their own address, because they will be msg.sender when they call the function.

2. Off-chain, they compute uint256(keccak256(abi.encodePacked(theirAddress, nextTimestamp, difficulty))) % 4 for the next few likely timestamps. The result tells them, in advance, which index would win if they called selectWinner() in that block.

3. If the result is not their own index, they simply do nothing and wait for a later block. They lose nothing by waiting, because calling is free apart from gas and anyone can call at any time after the duration ends.

4. When a block arrives in which the computed winner index is their own, they call selectWinner(). The contract computes the same hash with the same inputs, selects them, sends them 80% of the collected entrance fees, and mints them a puppy.

5. The same calculation applies to rarity: the attacker can check uint256(keccak256(abi.encodePacked(theirAddress, difficulty))) % 100 in advance, and call only when the result falls in the legendary range, above 95.

A block producer can go further, adjusting the timestamp within the allowed range or choosing which transactions to include, to change the hash outcome in their favour.

Recommended Mitigation

Do not use block.timestamp, block.difficulty, or msg.sender as sources of randomness. Use a verifiable random function, such as Chainlink VRF, so that the random value is generated off-chain and verified on-chain, and cannot be predicted or influenced by the caller or by a block producer. As a minimum, use a two-step commit-and-reveal scheme, so the inputs are fixed before the random value is revealed, and no single party can choose the outcome after seeing the inputs.