`refund()` leaves empty slots in `players`, so `selectWinner()` prices the prize pool off a stale player count — a single refund bricks small rounds and larger rounds overpay the prize
Summary
When a player refunds, refund() pays the fee back but only zeroes the array slot (players[playerIndex] = address(0)); the array length never shrinks. selectWinner() then computes the round's total as players.length * entranceFee, i.e. it counts refunded (empty) slots as if their money were still in the contract. The prize pool is therefore systematically larger than the balance backing it.
Vulnerability Details
In src/PuppyRaffle.sol:
Two consequences of the same root cause:
Small rounds are permanently bricked. With N players and r refunds, the real balance is (N−r) × entranceFee while the prize owed is 0.8 × N × entranceFee. Whenever r > 0.2N the prize exceeds the balance, the payout call at line 173 fails,
selectWinner()reverts,delete playersnever executes, and the round can never be drawn or advanced. Minimum case: N = 4, r = 1 — balance 3 ETH, prize 3.2 ETH. A single refund (which costs the player nothing; they get their own fee back) bricks the whole round, and this can be repeated every round. There is no admin recovery path (no pause, no forced draw, no array fix-up function). Additionally, ifwinnerIndexlands on a blanked slot, the winner isaddress(0)and_safeMintreverts, bricking the draw the same way.Larger rounds overpay. When r ≤ 0.2N the draw succeeds, but the winner is paid 80% of the inflated total. Verified in the PoC below: with 10 players and 1 refund, the winner received 8 ether (80% of 10) instead of 7.2 ether (80% of the 9 ether actually held), the contract kept 1 ether of real fees while
totalFeesrecorded 2 ether, andwithdrawFees()reverted afterwards because its strict balance equality can no longer hold.
Impact
Zero-cost, repeatable denial of the protocol's core function (drawing a winner, minting the NFT, collecting fees) after a single refund; in larger rounds, direct overpayment of the prize at the expense of the protocol's 20% fee share and corruption of fee accounting. Players can still exit individually via refund(), which is why a Medium reading is possible; the High reading rests on the core function being permanently and repeatably disabled with no recovery path, plus real funds being overpaid in the larger-round case.
Proof of Concept
Both scenarios verified locally with forge test:
Recommended Mitigation
Track the collected amount from actual funds, not from array length: keep a running totalAmountCollected adjusted on entry and refund, or compute the prize pool from the contract balance net of totalFees. Alternatively, remove refunded players from the array (swap-and-pop) so players.length always reflects active players.
Lead Judging Commences
[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() ```
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.