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

Refunded players remain in the array as address(0), corrupting selectWinner accounting and allowing an address(0) winner

alphalens

Root + Impact

  • refund() zeroes a player's slot but never shrinks the players array, so selectWinner() still counts the refunded slot. This inflates the prize pool above the contract's real balance and can select address(0) as the winner, either burning the prize or permanently reverting selectWinner().

Description

  • After the raffle ends, selectWinner() should pay out a prize equal to 80% of the ETH actually collected from the active players and mint the winner an NFT.

  • When a player calls refund(), their slot is set to address(0) but players.length is unchanged, so the refunded (and already reimbursed) player is still counted. selectWinner() derives totalAmountCollected from players.length, so the prize pool exceeds the real balance, and winnerIndex can land on an address(0) slot.

// in refund():
@> players[playerIndex] = address(0); // slot zeroed, but array length is NOT reduced
​
// in selectWinner():
@> uint256 totalAmountCollected = players.length * entranceFee; // still counts refunded slots
uint256 prizePool = (totalAmountCollected * 80) / 100; // > real balance
address winner = players[winnerIndex];
@> // winnerIndex may point to a refunded address(0) slot

Risk

Likelihood:

  • This occurs whenever at least one player refunds before the raffle is drawn, which is a normal, permitted user action.

  • The larger the share of refunded players, the more the prize pool overshoots the balance, guaranteeing the payout call or the NFT mint reverts.

Impact:

  • selectWinner() reverts (prizePool > balance, or _safeMint to address(0)), so the winner can never be drawn and the remaining ETH is permanently stuck.

  • If winnerIndex lands on an address(0) slot, the prize is sent to the zero address and burned.

Proof of Concept

  • Four players enter and one refunds. The contract now holds 3 * entranceFee, but selectWinner() still uses players.length == 4, so prizePool = 3.2 * entranceFee > balance. The draw therefore reverts and the raffle can never conclude.

function test_RefundBreaksSelectWinner() public {
address[] memory entrants = new address[](4);
entrants[0] = alice; entrants[1] = bob; entrants[2] = carol; entrants[3] = dave;
puppyRaffle.enterRaffle{value: entranceFee * 4}(entrants);
​
// Dave refunds; his slot becomes address(0) but players.length stays 4
uint256 idx = puppyRaffle.getActivePlayerIndex(dave);
vm.prank(dave);
puppyRaffle.refund(idx);
​
// Contract holds 3 * entranceFee, but selectWinner still uses players.length == 4
vm.warp(block.timestamp + duration + 1);
​
// prizePool = (4 * entranceFee * 80) / 100 = 3.2 * entranceFee > balance (3 * entranceFee)
vm.expectRevert();
puppyRaffle.selectWinner();
}

Recommended Mitigation

  • Do not leave holes in the array. On refund, remove the player by swapping in the last element and popping, and derive the collected amount from the real balance or an active-player counter rather than from players.length.

- players[playerIndex] = address(0);
+ players[playerIndex] = players[players.length - 1];
+ players.pop();
...
- uint256 totalAmountCollected = players.length * entranceFee;
+ uint256 totalAmountCollected = address(this).balance;
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 5 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-01] Potential Loss of Funds During Prize Pool Distribution

## Description In the `selectWinner` function, when a player has refunded and their address is replaced with address(0), the prize money may be sent to address(0), resulting in fund loss. ## Vulnerability Details In the `refund` function if a user wants to refund his money then he will be given his money back and his address in the array will be replaced with `address(0)`. So lets say `Alice` entered in the raffle and later decided to refund her money then her address in the `player` array will be replaced with `address(0)`. And lets consider that her index in the array is `7th` so currently there is `address(0)` at `7th index`, so when `selectWinner` function will be called there isn't any kind of check that this 7th index can't be the winner so if this `7th` index will be declared as winner then all the prize will be sent to him which will actually lost as it will be sent to `address(0)` ## Impact Loss of funds if they are sent to address(0), posing a financial risk. ## Recommendations Implement additional checks in the `selectWinner` function to ensure that prize money is not sent to `address(0)`

Support

FAQs

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

Give us feedback!