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

High 2 form

alutmoon

Root + Impact

Description

refund() blanks the player's slot (players[playerIndex] = address(0), line 103) but never shrinks the players array. selectWinner() then trusts the stale players.length for both winner selection and prize accounting (lines 128-136):

uint256 winnerIndex =
@> uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
@> uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;

This creates two failure modes after a single honest refund:

There is no recovery: withdrawFees() (line 158) requires address(this).balance == totalFees, which no longer holds after a refund, so the leftover ETH is locked forever. No privileged rescue function exists.

Risk

Likelihood: High — a single honest refund by any player triggers it; no attacker skill or special conditions are required.

Impact: High — one refund permanently bricks the raffle and locks all remaining player funds with no recovery path.

Proof of Concept

The Foundry test below demonstrates the full bricking flow:

function test_PoC_H2_RefundBricksSelectWinner() public {
address[] memory p = new address[](4);
p[0] = playerOne; p[1] = playerTwo; p[2] = playerThree; p[3] = playerFour;
puppyRaffle.enterRaffle{value: 4 ether}(p);
​
vm.prank(playerTwo);
puppyRaffle.refund(1); // players[1] = address(0), length still 4
​
vm.warp(block.timestamp + duration + 1);
vm.expectRevert();
puppyRaffle.selectWinner(); // bricked: prize accounting vs blank slot
​
vm.expectRevert("PuppyRaffle: There are currently players active!");
puppyRaffle.withdrawFees(); // bricked: balance (3 ETH) != totalFees (0)
}

Test assertions and measured result (forge test --match-test test_PoC_H2_RefundBricksSelectWinner):

Recommended Mitigation

Track the true active-player count (or swap-and-pop the refunded slot) and use it — not players.length — in selectWinner() accounting, while ensuring address(0) can never be selected as winner. With accounting matching actual funds, the prize pool can never exceed the balance, and with no blank slots the _safeMint revert path disappears — selectWinner() can always complete and withdrawFees()' balance check holds.

payable(msg.sender).sendValue(entranceFee);
- players[playerIndex] = address(0);
+ players[playerIndex] = players[players.length - 1];
+ players.pop();
emit RaffleRefunded(playerAddress);
Updates

Lead Judging Commences

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