[H-2] Predictable (weak) randomness in PuppyRaffle::selectWinner lets an attacker choose the winner and the puppy rarity

Risk

Likelihood: High — selectWinner is permissionless and the seed is fully public and computable in advance; an attacker simply calls only when the result favors them.

Impact: High — The attacker reliably wins the prize pool (80% of all entrance fees) and can force a legendary NFT rarity, completely breaking the raffle's fairness for honest participants.

Severity: High (High likelihood × High impact).

Description

PuppyRaffle::selectWinner derives both the winner and the puppy rarity from on-chain values that are public and/or controllable at call time, so the result is not random — it can be computed in advance.

All three seed inputs are knowable before the transaction is mined:

• msg.sender — chosen by the attacker (selectWinner has no access control, so anyone can call it from any address/contract).

• block.timestamp / block.difficulty — known for the current block, and a validator can additionally manipulate them.

Because the formula is public and deterministic, an attacker can compute winnerIndex (and rarity) off-chain or inside a contract before calling, and only call selectWinner when the result favors them (reverting otherwise).

Impact

• An attacker who has entered the raffle can guarantee that they win the prize pool (80% of all entrance fees) instead of leaving it to chance.

• The attacker can also force a legendary rarity for the minted NFT.

• The raffle's core fairness guarantee is broken; honest participants effectively cannot win against a motivated attacker.

Proof of Concept

The winner can be predicted before the draw using the exact same public formula. From test/WeakRandomnessPoC.t.sol:

1. 4 players enter and the raffle period ends.

2. Before calling selectWinner, compute winnerIndex = keccak256(msg.sender, block.timestamp, block.difficulty) % players.length and read players(winnerIndex).

3. Call selectWinner; previousWinner equals the predicted address — every time.

A weaponized version is a contract that computes the would-be winnerIndex, and calls selectWinner only if it points to the attacker's own player slot (revert otherwise), retrying over blocks until it wins.

Recommended Mitigation

Do not derive randomness from on-chain values (msg.sender, block.timestamp, block.difficulty/prevrandao). Use a verifiable, manipulation-resistant randomness source such as Chainlink VRF, which delivers a random word that callers and validators cannot predict or grind. Split selectWinner into a two-step request/fulfil pattern: one transaction requests randomness, and the VRF coordinator returns it in a callback where the winner is finalised.

Because randomWords[0] is produced off-chain by the VRF and verified on-chain, no participant (or validator) can compute the outcome in advance or selectively trigger a favourable draw.