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

[H-4] selectWinner() Can Permanently Freeze Raffle if Winning Index Lands on address(0) Slot

lmsand

Root + Impact

  • Root: refund() Leaves address(0) Gaps in Players Array

  • Impact: selectWinner() Permanently Reverts, Freezing All Player Funds

Description

  • The refund() function sets a refunded player's slot to address(0) instead of removing them from the array

The selectWinner() function selects a winner by index from the same array without checking for address(0)

  • If the randomly selected index lands on an address(0) slot, the contract attempts to send the prize pool to address(0) and mint an NFT to address(0)

  • Both operations revert, causing selectWinner() to revert entirely

  • Since delete players only runs after a successful selectWinner(), the array never resets

  • With no admin rescue function, all player funds are permanently frozen in the contract

// @audit address(0) slots can exist after refunds
address winner = players[winnerIndex];
​
// @audit no check that winner != address(0)
(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: Medium

  • Requires at least one player to have called refund() before selectWinner()

The more refunds that occur, the higher the probability of landing on an empty slot

  • No special permissions or attack needed — happens through normal protocol usage

Impact: High

  • All player funds are permanently frozen with no recovery mechanis

    • The raffle can never complete

    • Protocol must be fully redeployed to recover

    • All legitimate players lose their entrance fees

Proof of Concept

Step-by-step:

  1. 4 players enter the raffle

  2. One player calls refund(), leaving an address(0) slot in the array

  3. Raffle duration passes

  4. selectWinner() is called

  5. If winnerIndex lands on the address(0) slot, the call to send ETH reverts

  6. delete players never runs, funds are frozen forever

function test_selectWinnerWithZeroAddress() public {
// Enter 4 players
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 one refunds, leaving address(0) at index 0
uint256 playerIndex = puppyRaffle.getActivePlayerIndex(playerOne);
vm.prank(playerOne);
puppyRaffle.refund(playerIndex);
​
// Fast forward time
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
// If winnerIndex lands on index 0 (address(0)), selectWinner reverts
// We can manipulate this since randomness is weak (see H-2)
puppyRaffle.selectWinner();
assertEq(puppyRaffle.previousWinner(), address(0));
}

Recommended Mitigation

Replace the address(0) assignment in refund() with a swap-and-pop pattern to keep the array compact and free of empty slots:

- players[playerIndex] = address(0);
​
+ players[playerIndex] = players[players.length - 1];
+ players.pop();
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!