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

Potential Loss of Funds During Prize Pool Distribution: refunds leave phantom `players.length`, so `selectWinner` distributes more than collected

hopeh

Description

The intended behavior is that during prize-pool distribution the winner receives 80% of what the raffle actually collected and the remaining 20% is retained as fees, so the sum distributed never exceeds the funds held.

The specific issue is that selectWinner computes the distributed amounts from players.length, but refund removes a player's ETH from the contract while leaving their slot counted in players.length. The distribution is therefore based on funds that are no longer in the contract, so the prize pool pays out more than was collected and the fee portion becomes unbacked — funds are lost during prize-pool distribution.

// refund: the player's ETH leaves, but players.length is NOT reduced
payable(msg.sender).sendValue(entranceFee);
players[playerIndex] = address(0); // slot zeroed but still counted
​
// selectWinner distribution:
uint256 totalAmountCollected = players.length * entranceFee; // @> counts refunded players (phantom funds)
uint256 prizePool = (totalAmountCollected * 80) / 100; // @> larger than real funds held
uint256 fee = (totalAmountCollected * 20) / 100;
totalFees = totalFees + uint64(fee);
(bool success,) = winner.call{value: prizePool}(""); // @> overpays from the commingled balance

Because totalAmountCollected is padded by refunded slots, prizePool can exceed the ETH contributed by active players. The winner is paid from the single contract balance (which also holds accumulated totalFees), so the overpayment drains the fee reserve. The protocol loses funds during distribution, and withdrawFees (which needs balance == totalFees) can no longer succeed.

Risk

Likelihood: Medium

  • refund is a normal user action and always leaves a counted address(0) slot; any raffle with at least one refund distributes an inflated prize.

Impact: High

  • Loss of funds during prize-pool distribution: the winner is overpaid relative to funds collected, the fee reserve backing totalFees is drained, and fee withdrawal is permanently blocked.

Proof of Concept

// 5 players enter -> 5*fee held. 1 refunds -> 4*fee held, players.length still 5.
// selectWinner: prizePool = 80% of 5*fee = 4*fee -> winner is paid ALL remaining real funds;
// fee = 1*fee added to totalFees but 0 ETH left to back it -> fee funds lost; withdrawFees never passes.

Recommended Mitigation

Base the distribution on funds actually held, and remove refunded players (swap-and-pop) so players.length reflects only active players: uint256 totalAmountCollected = address(this).balance;.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 20 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!