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

Refunded ghost slots inflate the prize and corrupt fee accounting

nexus0ps

Description

The intended behavior: the prize and fee should be computed from the ETH that participants actually contributed.

The bug: refund() zeroes a player's slot but leaves the array length unchanged, so refunded entries (now address(0)) are still counted. selectWinner() computes the pot from players.length, so after any refund the prize and fee are overstated relative to the real balance.

// refund() leaves a ghost slot:
@> players[playerIndex] = address(0); // length unchanged
...
// selectWinner() counts ghost slots:
@> uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;

Risk

Likelihood: Medium. Any refund before the draw triggers the miscount; refunds are a normal, intended feature.

Impact: Medium. The winner is overpaid relative to real contributions, draining the contract and leaving totalFees recording fees that are not backed by ETH, so withdrawFees() reverts. (Separately, if the random index lands on a refunded address(0) slot, _safeMint(address(0)) reverts and freezes the draw.)

Proof of Concept

Run forge test --match-test test_M4_refund_inflates_prize_and_breaks_accounting. Five players enter (5 ETH); one refunds, leaving 4 ETH. The pot is still computed as 5 ETH, so the winner receives 4 ETH (the entire balance), the contract is drained to 0, yet totalFees records 1 ETH of phantom fees, and withdrawFees() then reverts.

function test_M4_refund_inflates_prize_and_breaks_accounting() public {
_enterFresh(5, 1); // 5 ETH in the pot
vm.prank(address(5));
raffle.refund(4); // ghost slot; real balance now 4 ETH
​
uint256 ts = _winningTimestamp(address(this), 0, 5, raffle.raffleStartTime() + duration);
vm.warp(ts);
raffle.selectWinner(); // pays 80% of inflated 5 ETH = 4 ETH
​
assertEq(address(raffle).balance, 0); // contract drained
assertEq(uint256(raffle.totalFees()), 1 ether); // phantom, unbacked fees
vm.expectRevert("PuppyRaffle: There are currently players active!");
raffle.withdrawFees();
}

Recommended Mitigation

Track an active-player count (or compact the array on refund) and base the prize/fee math on the funds actually held rather than on players.length.

- uint256 totalAmountCollected = players.length * entranceFee;
+ uint256 totalAmountCollected = activePlayers * entranceFee; // decremented on each refund

Decrement activePlayers inside refund() so the pot always reflects real contributions, and skip address(0) slots when selecting the winner.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours 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!