Puppy Raffle

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

`selectWinner()` Does Not Skip Refunded (address(0)) Slots — Prize Pool Sent to Null Address, Funds Burned

selectWinner() Does Not Skip Refunded (address(0)) Slots — Prize Pool Sent to Null Address, Funds Burned

Description

When a player calls refund(), their slot in the players array is replaced with address(0) rather than being removed. The selectWinner() function subsequently picks a winner by computing an index into the players array without excluding address(0) entries. If the winning index resolves to a refunded (zeroed) slot, the call winner.call{value: prizePool}("") sends the entire prize pool to the zero address — permanently burning those funds.

A call to address(0).call{value: prizePool}("") on the EVM succeeds and returns (true, "") without reverting, making the loss silent and unrecoverable.

function selectWinner() external {
// ...
uint256 winnerIndex = uint256(...) % players.length;
@> address winner = players[winnerIndex]; // Could be address(0) after refund()
// No check: require(winner != address(0), ...)
uint256 prizePool = (totalAmountCollected * 80) / 100;
// ...
@> (bool success,) = winner.call{value: prizePool}(""); // ETH sent to 0x0 — burned
require(success, "...");
}

Risk

Likelihood: High

  • This occurs naturally whenever any player calls refund() before selectWinner() — a common and expected protocol flow.

  • The probability increases with the refund rate and with targeted array manipulation by adversaries who know the RNG output in advance (compounding with D02).

Impact: High

  • The entire prize pool (80% of all entrance fees) is permanently lost to the zero address with no recovery mechanism.

  • Legitimate participants who did NOT refund receive nothing despite having paid.

Severity: High

Proof of Concept

The following scenario demonstrates the bug through a normal usage path without any sophisticated attack:

  1. 4 players enter the raffle (minimum required).

  2. Player at index 2 calls refund()players[2] = address(0).

  3. selectWinner() is called; the RNG returns index 2.

  4. winner = players[2] = address(0).

  5. address(0).call{value: prizePool}("") returns (true, "") — ETH burned.

  6. _safeMint(address(0), tokenId) reverts on OZ ERC721 (recipient cannot be zero) — the transaction REVERTS entirely, leaving the raffle in a stuck state.

// Simplified trace
address[] memory players = [addr1, addr2, address(0), addr4];
// winnerIndex = 2
address winner = players[2]; // address(0)
(bool ok,) = winner.call{value: prizePool}(""); // ok=true, ETH burned
_safeMint(address(0), 0); // REVERTS — OZ ERC721 rejects address(0) recipient
// Entire selectWinner() tx reverts — raffle is stuck, cannot complete

Recommended Mitigation

Validate that the selected winner is not address(0) before proceeding. Alternatively, compact the players array on refund instead of zeroing slots:

uint256 winnerIndex = uint256(...) % players.length;
address winner = players[winnerIndex];
+require(winner != address(0), "PuppyRaffle: Winner is a refunded player — re-draw");

A more complete fix is to remove refunded players from the array rather than zero their slot, ensuring the array contains only active participants at all times. This also reduces players.length for future gas calculations.

Updates

Lead Judging Commences

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