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

[H-4] Denial of Service via Zero-Address Winner Selection After Refunds

0xaljzoli

[H-4] Denial of Service via Zero-Address Winner Selection After Refunds

Title: Denial of Service via Zero-Address Winner Selection After Refunds

Impact: High

Likelihood: Medium

Description

  • The selectWinner function randomly selects a winner index from the players array and mints an NFT to that address. The refund function sets a player's slot to address(0) rather than removing the entry, leaving zero-address "holes" in the array.

  • When selectWinner picks an index corresponding to a refunded player, it attempts to mint an NFT to address(0). OpenZeppelin's ERC721 _safeMint function contains an internal check require(to != address(0), "ERC721: mint to the zero address"), which causes the entire selectWinner transaction to revert. As more players refund, the probability of selecting a zero-address increases, making the raffle progressively harder to close.

function refund(uint256 playerIndex) public {
// ...
players[playerIndex] = address(0); // @> Leaves address(0) hole in array
}
​
function selectWinner() external {
// ...
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex]; // @> Could be address(0) if slot was refunded
​
// ...
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
- _safeMint(winner, tokenId); // @> Reverts here if winner == address(0)
}

Risk

Likelihood:

  • Any player can call refund to set their slot to address(0). After just one refund out of 4 players, there is a 25% chance that selectWinner picks the zero-address and reverts. With 2 refunds out of 4 players, the probability rises to 50%, and with 3 refunds, the function will almost certainly revert.

  • An attacker could intentionally enter the raffle with multiple addresses and refund them all to intentionally inflate the number of zero-address slots, making the DoS condition nearly certain.

Impact:

  • When selectWinner reverts due to a zero-address winner, the raffle cannot be completed. The ETH prize pool remains locked in the contract, and no NFT is minted. All legitimate players who did not refund are unable to receive their winnings.

  • The raffleStartTime is updated only after a successful selectWinner call, so the raffle duration does not reset on reverts. However, since the function keeps reverting, the raffle is permanently stuck and no new round can begin.

Proof of Concept

function testDoSFromRefundedWinner() public {
// 4 players enter the raffle
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 — their slot becomes address(0)
uint256 indexOfTwo = puppyRaffle.getActivePlayerIndex(playerTwo);
vm.prank(playerTwo);
puppyRaffle.refund(indexOfTwo);
​
// Raffle duration passes
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
// selectWinner now has a 25% chance of picking address(0)
// When it does, _safeMint(address(0), tokenId) reverts
// The raffle is stuck — no winner can ever be selected
vm.expectRevert(); // _safeMint to zero address
puppyRaffle.selectWinner();
}

This PoC shows that after a single refund, selectWinner can revert when the random index lands on the refunded slot. The _safeMint call fails because OpenZeppelin's ERC721 implementation explicitly prohibits minting to address(0). Since the winner index is determined by a hash, the attack cannot be predicted in advance — it is probabilistic, but as more players refund, the likelihood of the DoS approaches certainty. An attacker can intentionally refund multiple entries to guarantee the function never succeeds.

Recommended Mitigation

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 is a refunded player");
uint256 totalAmountCollected = players.length * entranceFee;
// ...
}

Adding a require(winner != address(0)) check prevents the function from reverting deep inside _safeMint. However, this is a partial fix — the better long-term solution is to compact the players array in refund by swapping the removed entry with the last element and popping, which eliminates zero-address holes entirely and keeps the array dense:

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");
​
- players[playerIndex] = address(0);
+ players[playerIndex] = players[players.length - 1];
+ players.pop();
​
payable(msg.sender).sendValue(entranceFee);
emit RaffleRefunded(playerAddress);
}

This swap-and-pop approach removes the zero-address slot entirely, keeping only active players in the array and eliminating the root cause of this vulnerability.

Updates

Lead Judging Commences

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