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

selectWinner() Can Select address(0) as Winner

dparaste48

Root + Impact

When players refund, their address in the players array is set to address(0). The selectWinner() function does not validate that the selected winner is not address(0), which causes the transaction to revert when attempting to send ETH to the zero address, permanently locking the raffle.

Description

Describe the normal behavior in one or more sentences:

  • A legitimate active player should be selected as the winner when selectWinner() is called

  • The winner should receive the prize pool (80% of total funds) and be minted an NFT

Explain the specific issue or problem in one or more sentences:

  • The refund() function sets players[playerIndex] = address(0) when a player refunds

  • The selectWinner() function randomly selects an index from the players array without checking if that address is address(0)

  • If address(0) is selected, the call to send ETH fails, causing the entire transaction to revert and permanently locking the raffle

// @ src/PuppyRaffle.sol:96-105 (refund sets address(0))
function refund(uint256 playerIndex) public {
address playerAddress = players[playerIndex];
require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund");
require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active");
​
payable(msg.sender).sendValue(entranceFee);
players[playerIndex] = address(0); // Creates address(0) in array
emit RaffleRefunded(playerAddress);
}
​
// @ src/PuppyRaffle.sol:125-154 (selectWinner doesn't check for address(0))
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]; // Could be address(0)
// No validation here
(bool success,) = winner.call{value: prizePool}(""); // Fails when winner is address(0)
require(success, "PuppyRaffle: Failed to send prize pool to winner");
_safeMint(winner, tokenId);
}

Risk

Likelihood:
Reason 1: Occurs whenever a refunded player's slot is randomly selected
Reason 2: Probability increases with each refund (if 1 out of 4 players refunds, there is a 25% chance)

Impact:
Impact 1: Raffle becomes permanently locked, preventing winner selection and trapping all funds
Impact 2: All participants lose their entrance fees as funds cannot be recovered without contract upgrade

Proof of Concept

Place the following test in test/AuditTest.t.sol:

function testSelectWinnerAddress0() public {
address[] memory players = new address[](4);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
players[3] = playerFour;
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
​
// Player 2 refunds
uint256 player2Index = puppyRaffle.getActivePlayerIndex(playerTwo);
vm.prank(playerTwo);
puppyRaffle.refund(player2Index);
// Now players[1] = address(0)
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
// Calculate which index will be selected
uint256 winnerIndex = uint256(keccak256(abi.encodePacked(address(this), block.timestamp, block.difficulty))) % 4;
// If winner is address(0), selectWinner will fail
if (winnerIndex == 1) {
vm.expectRevert("PuppyRaffle: Failed to send prize pool to winner");
puppyRaffle.selectWinner();
console.log("BUG CONFIRMED: address(0) was selected, transaction reverted!");
} else {
puppyRaffle.selectWinner();
console.log("Winner index was", winnerIndex, "- rerun to hit address(0) case");
}
}

Run with: forge test --mt testSelectWinnerAddress0 -vv

Recommended Mitigation

Add validation to ensure the winner is not address(0):

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];
+ require(winner != address(0), "PuppyRaffle: Winner cannot be address(0)");
// ... rest of function
}
Updates

Lead Judging Commences

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