Puppy Raffle

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

CRIT-02: Winner Can Be Zero Address After Refunds

CRIT-02: Winner Can Be Zero Address After Refunds

Description

The refund() function sets players[playerIndex] = address(0) but does not remove the element from the array. When selectWinner() picks a random index using players.length, it seems the user wants me to continue creating the array, it can land on a refunded (zeroed) slot, making address(0) the winner.

The prize pool (80% of total) is sent to address(0) (burned), and an NFT is minted to address(0).

// Root cause in the codebase with @> marks to highlight the relevant section
function refund(uint256 playerIndex) public {
@> players[playerIndex] = address(0);
// ...
}
function selectWinner() external {
// ...
@> uint256 winnerIndex = uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
@> address winner = players[winnerIndex];
}

Risk

Likelihood:

  • Occurs whenever any player has called refund() before selectWinner()

  • Probability increases with number of refunds

Impact:

  • Prize pool permanently burned (sent to zero address)

  • NFT minted to zero address (unrecoverable)

  • Protocol loses funds and NFTs

  • Winner selection fundamentally broken

Proof of Concept

// 1. 10 players enter raffle
// 2. Players at indices 0, 3, 7 call refund()
// 3. players array now has address(0) at indices 0, 3, 7
// 4. selectWinner() called, winnerIndex = 3 (for example)
// 5. winner = players[3] = address(0)
// 6. prizePool sent to address(0) - BURNED
// 7. NFT minted to address(0)

Recommended Mitigation

function selectWinner() external {
// ... existing checks ...
// Build array of active players (non-zero addresses)
address[] memory activePlayers = new address[](players.length);
uint256 activeCount = 0;
for (uint256 i = 0; i < players.length; i++) {
if (players[i] != address(0)) {
activePlayers[activeCount] = players[i];
activeCount++;
}
}
require(activeCount >= 4, "PuppyRaffle: Need at least 4 active players");
- uint256 winnerIndex = uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
- address winner = players[winnerIndex];
+ uint256 winnerIndex = uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % activeCount;
+ address winner = activePlayers[winnerIndex];
// ... rest of function ...
}
Updates

Lead Judging Commences

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