Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

`refund()` leaves `address(0)` holes → `selectWinner()` prize pool calculation ignores refunds and always reverts

aiada

Description

  • Normal: After players refund, selectWinner() should calculate the prize pool based on actual remaining players, not the original count.

  • Bug: prizePool is calculated as (players.length * entranceFee * 80) / 100. But players.length never decreases after refunds (players are set to address(0), not removed). The contract holds less ETH than the calculated prize pool, so the winner.call{value: prizePool} always reverts.

// src/PuppyRaffle.sol:131-152
uint256 totalAmountCollected = players.length * entranceFee; //@> Counts address(0) as players!
uint256 prizePool = (totalAmountCollected * 80) / 100; //@> Over-calculates — refunded ETH is gone
// ...
(bool success,) = winner.call{value: prizePool}(""); //@> REVERTS — insufficient balance
require(success, "PuppyRaffle: Failed to send prize pool to winner");

This is distinct from H-01 (which focuses on gas waste and zero-address winner). Here the issue is that even if a valid non-zero address is selected as winner, the contract doesn't have enough ETH to pay the prize.

Risk

Likelihood:

  • Any refund creates the condition — only 1 refund needed to cause revert

  • No attacker needed — normal protocol usage triggers the bug

  • Users who are unhappy with their odds naturally refund, making this inevitable

Impact:

  • selectWinner() permanently reverts after any refund occurs

  • Raffle is bricked — no winner selected, no NFT minted

  • Players can still refund (lucky), but the protocol's core function is destroyed

Proof of Concept

function testWinnerSelectionRevertsAfterExit() public playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
// 4 players entered. Player 4 refunds.
vm.prank(playerFour);
puppyRaffle.refund(3);
​
// selectWinner reverts — prizePool > contract balance
vm.expectRevert();
puppyRaffle.selectWinner();
​
// Even if we add extra ETH, it still reverts because winner may be address(0)
vm.deal(address(puppyRaffle), 10 ether);
vm.expectRevert("ERC721: mint to the zero address");
puppyRaffle.selectWinner();
}

Run with:

forge test --match-test testWinnerSelectionRevertsAfterExit -vvvv

Recommended Mitigation

function refund(uint256 playerIndex) public {
address playerAddress = players[playerIndex];
require(playerAddress == msg.sender);
require(playerAddress != address(0));
​
+ // Swap-and-pop: remove the player entirely, keeping length accurate
+ players[playerIndex] = players[players.length - 1];
+ players.pop();
payable(msg.sender).sendValue(entranceFee);
- players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 21 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!