Puppy Raffle

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

Refunded players remain as `address(0)` in `players`, so `selectWinner` over-counts the prize pool and can pick a zeroed slot (burned prize + mint revert DoS)

Root + Impact

Description

  • After a refund the player should no longer count toward the prize pool or be eligible to win.

  • refund only sets the slot to address(0) without shrinking players. selectWinner still uses players.length to size the pool and can select a zeroed index as the winner.

// refund leaves a blank slot:
@> players[playerIndex] = address(0);
...
// selectWinner over-counts and may pick a zero slot:
@> uint256 totalAmountCollected = players.length * entranceFee; // includes refunded slots
@> address winner = players[winnerIndex]; // may be address(0)
...
@> _safeMint(winner, tokenId); // reverts if winner == address(0)

Risk

Likelihood:

  • Occurs whenever at least one player has refunded before selectWinner and the random index lands on a zeroed slot (probability grows with the number of refunds).

Impact:

  • totalAmountCollected exceeds the real balance, so prize/fee math can exceed available ETH; if the winner is address(0), the prize ETH is sent to the zero address (burned) and _safeMint(address(0), ...) reverts, permanently DoS'ing selectWinner for that draw.

Proof of Concept

// 5 players enter; player[2] refunds (players[2] = address(0)).
// players.length is still 5, so totalAmountCollected = 5 * entranceFee (overcount).
// If winnerIndex resolves to 2: winner = address(0);
// winner.call{value: prizePool}("") sends ETH to address(0) (burned),
// then _safeMint(address(0), tokenId) reverts -> selectWinner reverts.

Recommended Mitigation

function refund(uint256 playerIndex) public {
...
- players[playerIndex] = address(0);
+ // remove the slot instead of leaving a blank (swap-and-pop)
+ players[playerIndex] = players[players.length - 1];
+ players.pop();
emit RaffleRefunded(playerAddress);
}

(Or track an active-player count and require winner != address(0); size the pool from active players only. Note: swap-pop changes indices, so pair it with a mapping-based lookup.)

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 6 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!