Puppy Raffle

Refunded Player Slot Becomes address(0) and Can Freeze the Raffle Forever

Severity: Medium

Description

• Players call refund() to exit the raffle before a winner is selected. Their slot in the players array is set to address(0) rather than being removed.

• If selectWinner() draws the index of a refunded slot, winner equals address(0). The ETH prize is sent to address(0) burning it, then _safeMint(address(0),
  tokenId) reverts because ERC721 forbids minting to the zero address. This causes selectWinner() to permanently revert, freezing all remaining player funds with
  no recovery path.

function refund(uint256 playerIndex) public {
// @> Slot set to address(0) instead of being removed
players[playerIndex] = address(0);
}

function selectWinner() external {
address winner = players[winnerIndex];
// @> If winnerIndex points to refunded slot, winner = address(0)
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
// @> _safeMint to address(0) always reverts — raffle frozen forever
_safeMint(winner, tokenId);
}

Risk

Likelihood:

• Any raffle round where at least one player has refunded carries a nonzero probability of drawing a zero-address slot proportional to the number of refunds.

• With multiple refunds the probability becomes significant — only one frozen selectWinner() call is needed to permanently lock all funds.

Impact:

• selectWinner() becomes permanently uncallable — all remaining players' entrance fees are locked in the contract with no withdrawal mechanism.

• The raffle protocol is completely non-functional until redeployment.

Proof of Concept

After a player refunds, their slot becomes address(0). If that index is drawn as the winner, _safeMint(address(0), tokenId) reverts and the raffle is
permanently frozen with all funds locked inside:

function test_WinnerAddressZero() public {
address p0 = makeAddr("p0"); address p1 = makeAddr("p1");
address p2 = makeAddr("p2"); address p3 = makeAddr("p3");
address[] memory players = new address;
players[0] = p0; players[1] = p1;
players[2] = p2; players[3] = p3;
vm.deal(address(this), 4 ether);
raffle.enterRaffle{value: 4 ether}(players);
vm.prank(p0);
raffle.refund(0);
assertEq(raffle.players(0), address(0));
// If selectWinner draws index 0:
// _safeMint(address(0), tokenId) reverts
// All remaining funds permanently locked
}

Recommended Mitigation

Use the swap-and-pop pattern on refund to cleanly remove the player slot and avoid zero-address entries in the array entirely:

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");                                                                             

• players[playerIndex] = players[players.length - 1];                                                                                                       
  

• players.pop();                                                                                                                                            
  payable(msg.sender).sendValue(entranceFee);                                                                                                               
  

• players[playerIndex] = address(0);                                                                                                                        
  emit RaffleRefunded(playerAddress);
  

  }