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

Winner can be zero address

fyuryy

PuppyRaffle::selectWinner can select a refunded player (address(0)) as the winner

Description

When a player calls refund, their slot in the players array is set to address(0) rather than being removed. However, selectWinner does not check whether the selected winner address is address(0). If the randomly selected index points to a refunded player's slot, the prize pool ETH is sent to the zero address (burned) and the NFT mint to address(0) will revert in _safeMint, causing the entire transaction to revert.

function selectWinner() external {
// ...
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
@> address winner = players[winnerIndex]; // Can be address(0) if player refunded
// ...
@> (bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
@> _safeMint(winner, tokenId); // Reverts if winner is address(0)
}

Risk

Likelihood:

  • Any player can refund at any time before the raffle ends, creating address(0) slots in the array

  • The more refunds that occur, the higher the probability that selectWinner picks a zeroed-out slot

  • With many refunds, it may become impossible to conclude the raffle at all (every call reverts)

Impact:

  • _safeMint to address(0) reverts, causing the entire selectWinner transaction to fail

  • If many slots are zeroed out, the raffle can become permanently stuck, locking all remaining funds

  • Even if it doesn't revert (hypothetically), the prize ETH would be sent to address(0) and burned

Proof of Concept

  1. Four players enter the raffle normally, occupying indices 0 through 3 in the players array.

  2. Three of the four players (addresses 1, 2, and 3) call refund, which sets their slots to address(0). The players array is now [address(0), address(0), address(0), address(4)].

  3. We warp time past the raffle duration so selectWinner can be called.

  4. The winner index is computed as keccak256(...) % players.length, where players.length is still 4 (refunds don't shrink the array).

  5. There is a 75% chance (indices 0, 1, or 2) that the selected winner is address(0). When this happens, _safeMint(address(0), tokenId) reverts because ERC721 does not allow minting to the zero address.

  6. The raffle becomes stuck: selectWinner keeps reverting for most random outcomes, and only succeeds on the 25% chance that index 3 is selected. With even more refunds, the probability of success drops further, potentially locking the raffle permanently.

function testWinnerCanBeZeroAddress() public {
address[] memory players = new address[](4);
players[0] = address(1);
players[1] = address(2);
players[2] = address(3);
players[3] = address(4);
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
// Players 1, 2, and 3 refund
vm.prank(address(1));
puppyRaffle.refund(0);
vm.prank(address(2));
puppyRaffle.refund(1);
vm.prank(address(3));
puppyRaffle.refund(2);
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
// 3 out of 4 slots are address(0). If the random index hits any of them,
// selectWinner will revert on _safeMint(address(0), ...).
// Repeated calls with different block parameters will keep reverting
// until index 3 is selected (only 25% chance per attempt).
}

Recommended Mitigation

Add a check to ensure the selected winner is not address(0), and re-roll or revert with a meaningful error:

function selectWinner() external {
// ...
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
+ require(winner != address(0), "PuppyRaffle: Winner is not an active player");
// ...
}

A more robust solution would be to maintain a separate array of only active players, removing entries on refund rather than zeroing them out.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 1 day ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-04] `PuppyRaffle::refund` replaces an index with address(0) which can cause the function `PuppyRaffle::selectWinner` to always revert

## Description `PuppyRaffle::refund` is supposed to refund a player and remove him from the current players. But instead, it replaces his index value with address(0) which is considered a valid value by solidity. This can cause a lot issues because the players array length is unchanged and address(0) is now considered a player. ## Vulnerability Details ```javascript players[playerIndex] = address(0); @> uint256 totalAmountCollected = players.length * entranceFee; (bool success,) = winner.call{value: prizePool}(""); require(success, "PuppyRaffle: Failed to send prize pool to winner"); _safeMint(winner, tokenId); ``` If a player refunds his position, the function `PuppyRaffle::selectWinner` will always revert. Because more than likely the following call will not work because the `prizePool` is based on a amount calculated by considering that that no player has refunded his position and exit the lottery. And it will try to send more tokens that what the contract has : ```javascript uint256 totalAmountCollected = players.length * entranceFee; uint256 prizePool = (totalAmountCollected * 80) / 100; (bool success,) = winner.call{value: prizePool}(""); require(success, "PuppyRaffle: Failed to send prize pool to winner"); ``` However, even if this calls passes for some reason (maby there are more native tokens that what the players have sent or because of the 80% ...). The call will thankfully still fail because of the following line is minting to the zero address is not allowed. ```javascript _safeMint(winner, tokenId); ``` ## Impact The lottery is stoped, any call to the function `PuppyRaffle::selectWinner`will revert. There is no actual loss of funds for users as they can always refund and get their tokens back. However, the protocol is shut down and will lose all it's customers. A core functionality is exposed. Impact is high ### Proof of concept To execute this test : forge test --mt testWinnerSelectionRevertsAfterExit -vvvv ```javascript function testWinnerSelectionRevertsAfterExit() public playersEntered { vm.warp(block.timestamp + duration + 1); vm.roll(block.number + 1); // There are four winners. Winner is last slot vm.prank(playerFour); puppyRaffle.refund(3); // reverts because out of Funds vm.expectRevert(); puppyRaffle.selectWinner(); vm.deal(address(puppyRaffle), 10 ether); vm.expectRevert("ERC721: mint to the zero address"); puppyRaffle.selectWinner(); } ``` ## Recommendations Delete the player index that has refunded. ```diff - players[playerIndex] = address(0); + players[playerIndex] = players[players.length - 1]; + players.pop() ```

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!