Refunded players leave address(0) holes that can permanently block selectWinner()
Root Cause
refund() leaves holes (address(0)) in the players[] array, and selectWinner() picks a random index over the full array including holes:
@ = address(0); // refund() leaves a hole
@ winnerIndex = uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length; // may pick the hole
If winnerIndex lands on a hole, winner is address(0). The winner.call{value: prizePool} succeeds (address(0) accepts ETH), so the prize is burned; then _safeMint(address(0), tokenId) reverts, so the whole selectWinner() call reverts and the raffle can never complete.
Description
A player who refunds leaves a permanent hole in the array. The raffle can only be resolved by selectWinner(), which reverts whenever the random index lands on a hole. Because the index is a pure function of attacker-influenced block values, an attacker can simply call selectWinner() on blocks where the index hits the hole, guaranteeing the raffle never completes. The prize and all entrance fees stay locked forever (players can only self-refund).
Risk
Reason 1: array holes are never compacted and remain inside players.length.
Reason 2: _safeMint to address(0) reverts, blocking the winner payout, while the ETH transfer to address(0) succeeds.
Impact 1: permanent DoS of the raffle - all funds locked forever.
Impact 2: prize pool burned to address(0) (unrecoverable).
Proof of Concept
vm.prank(alice);
raffle.refund(0); // players[0] becomes address(0)
vm.warp(block.timestamp + 86400);
vm.prevrandao(0); // forces winnerIndex = 0, the hole
raffle.selectWinner(); // reverts - raffle permanently stuck
Forge test test_BUG3_refundedSlotCanBlockRaffle PASS: selectWinner() reverts and the raffle can never finish.
Recommended Mitigation
Compact the array on refund (swap the last element into the hole and pop) so players.length stays accurate and no zero slots are ever selected. Skipping zero winners alone is insufficient because the prize would still be burned - combine compaction with a winner-skip. Optionally restrict selectWinner() callers or enforce a timeout so a griefer cannot stall the raffle indefinitely.
Lead Judging Commences
[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() ```
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.