Puppy Raffle

AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Severity: high
Valid

Incorrect Use of players.length Instead of Active Player Count Leads to Fund Misallocation and Burn Risk

Root + Impact

  • refund() sets players[playerIndex] = address(0) but does not reduce array length.

  • Both conditions rely on players.length, which includes refunded slots.

  • As a result:

    Minimum player requirement can be bypassed with inactive entries.

    Prize pool calculation overestimates funds, misallocating ETH.

    RNG may select address(0) as winner, bricking the raffle

Description

The contract uses players.length to determine raffle state, minimum player requirements, and prize pool size. However, refunded players are set to address(0) and remain in the array. This means players.length counts inactive slots, leading to incorrect logic and potential protocol failure.

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

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

  • Reason 2

Impact:

  • Prize pool miscalculation: Winner may receive more or less ETH than intended.

  • Fee misallocation: Protocol fees derived from inflated pool size are incorrect.

  • Denial of service: Raffle can revert if zero address is selected as winner

Proof of Concept

  1. Four players enter the raffle

  2. Three players call refund()

  3. players array becomes: [ address(0), address(0), address(0), Alice ]

  4. players.length == 4 → requirement passes

  5. Winner selection has a 75% chance of selecting address(0)

  6. Funds and NFT may be permanently lost

Recommended Mitigation

  • Track active participants separately.

  • Alternatively, compact the array on refund (swap‑and‑pop) to remove inactive slots.

  • Ensure RNG skips address(0) entries.

- remove this code
+ add this code
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-01] Potential Loss of Funds During Prize Pool Distribution

## Description In the `selectWinner` function, when a player has refunded and their address is replaced with address(0), the prize money may be sent to address(0), resulting in fund loss. ## Vulnerability Details In the `refund` function if a user wants to refund his money then he will be given his money back and his address in the array will be replaced with `address(0)`. So lets say `Alice` entered in the raffle and later decided to refund her money then her address in the `player` array will be replaced with `address(0)`. And lets consider that her index in the array is `7th` so currently there is `address(0)` at `7th index`, so when `selectWinner` function will be called there isn't any kind of check that this 7th index can't be the winner so if this `7th` index will be declared as winner then all the prize will be sent to him which will actually lost as it will be sent to `address(0)` ## Impact Loss of funds if they are sent to address(0), posing a financial risk. ## Recommendations Implement additional checks in the `selectWinner` function to ensure that prize money is not sent to `address(0)`

Support

FAQs

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

Give us feedback!