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

Winner can be address(0) after refund, losing prize pool permanently

alaaeldinsabib

Summary

The selectWinner function can select address(0) as the winner when a player has been refunded. This causes the prize pool to be sent to address(0) and permanently lost, while the NFT is also minted to address(0), making both unrecoverable.

Description

When a player calls refund, their slot in the players array is set to address(0) instead of being removed. The selectWinner function uses players.length to calculate the winner index, which includes these zero-address slots. If the random selection picks a zero-address slot, the prize is sent to address(0) and lost forever.

This creates a critical issue: users who paid to enter the raffle lose their chance to win, and the prize pool is permanently destroyed.

Root Cause

File: src/PuppyRaffle.sol (lines 127-130, 140)

uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex]; // Can be address(0)
// ...
(bool success,) = winner.call{value: prizePool}(""); // ❌ Sends to address(0)

Risk

Severity: High
Likelihood: Medium
Impact: High

  • ❌ Prize pool sent to address(0) and lost forever

  • ❌ NFT minted to address(0) (unrecoverable)

  • ❌ Users lose their potential winnings

  • ❌ Requires any refund before winner selection

Proof of Concept

Scenario: A player refunds their ticket, leaving address(0) in the players array. The random selection then picks this zero-address slot as the winner.

function test_WinnerCanBeAddressZero() public {
// Step 1: Setup - 4 players enter
address[] memory players = new address[](4);
players[0] = address(1);
players[1] = address(2);
players[2] = address(3);
players[3] = address(4);
raffle.enterRaffle{value: entranceFee * 4}(players);
// Step 2: Player at index 2 refunds
vm.prank(address(3));
raffle.refund(2);
// Verify player slot is now address(0)
assertEq(raffle.players(2), address(0));
vm.warp(block.timestamp + duration + 1);
// Step 3: Brute-force to find msg.sender that selects index 2
address winningCaller;
for (uint256 i = 0; i < 100; i++) {
address caller = address(uint160(i + 10000));
uint256 winnerIndex = uint256(keccak256(abi.encodePacked(
caller,
block.timestamp + duration + 1,
block.difficulty
))) % 4;
if (winnerIndex == 2) {
winningCaller = caller;
break;
}
}
// Step 4: Call selectWinner with the manipulated caller
vm.prank(winningCaller);
raffle.selectWinner();
// Step 5: Verify winner is address(0) and prize is lost
assertEq(raffle.previousWinner(), address(0));
console.log("VULNERABILITY: Winner is address(0), prize lost forever!");
}

Test Output:

VULNERABILITY: Winner is address(0), prize lost forever!

What This Proves:

  1. ✅ Winner can be address(0) after refund

  2. ✅ Prize pool is lost forever

  3. ✅ NFT minted to address(0)

Recommended Mitigation

Skip zero-address slots when selecting winner:

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];
// ✅ Skip zero-address slots
while (winner == address(0)) {
winnerIndex = (winnerIndex + 1) % players.length;
winner = players[winnerIndex];
}
require(winner != address(0), "PuppyRaffle: No active players");
// ... rest of logic
}

Alternative (Better): Remove refunded players from the array:

function refund(uint256 playerIndex) public {
// ... validation ...
// Remove player by swapping with last element
players[playerIndex] = players[players.length - 1];
players.pop();
payable(msg.sender).sendValue(entranceFee);
emit RaffleRefunded(playerAddress);
}

Why This Fixes It:

  1. ✅ Zero-address slots are skipped during winner selection

  2. ✅ Prize is always sent to a valid player

  3. ✅ No funds lost to address(0)

References

  • CWE-672: Operation on a Resource after Expiration or Release

  • SWC-114: Transaction Order Dependence

Updates

Lead Judging Commences

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