Puppy Raffle

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

A winner that rejects ETH permanently bricks selectWinner()

Description

The intended behavior: selectWinner() should always be able to conclude a round, pay the winner, and reset the raffle.

The bug: the prize is pushed to the winner with a low-level call guarded by require(success). If the selected winner is a contract that reverts on receipt (no payable receive/fallback, or a deliberate revert), the call fails and the whole transaction reverts. Because the earlier state changes (delete players, timer reset) roll back with it, the contest gets stuck: the duration has passed and there are 4+ players, but selectWinner() reverts on every call.

delete players;
raffleStartTime = block.timestamp;
previousWinner = winner;
@> (bool success,) = winner.call{value: prizePool}("");
@> require(success, "PuppyRaffle: Failed to send prize pool to winner");
_safeMint(winner, tokenId);

Risk

Likelihood: Medium. It requires a contract winner that rejects ETH; combined with the weak randomness finding, an attacker can guarantee their reverting contract is the winner.

Impact: Medium. Denial of service on the core draw, locking all participants' funds with no recovery path.

Proof of Concept

Run forge test --match-test test_M2_reverting_winner_bricks_selectWinner. A contract with a reverting fallback enters as a player; the test grinds a timestamp so that contract is selected, then selectWinner() reverts with "Failed to send prize pool to winner".

function test_M2_reverting_winner_bricks_selectWinner() public {
RevertingWinner bomb = new RevertingWinner(); // fallback() external { revert(); }
address[] memory ps = new address[](4);
ps[0] = address(bomb); ps[1] = address(2); ps[2] = address(3); ps[3] = address(4);
raffle.enterRaffle{value: 4 ether}(ps);
// weak randomness lets us grind a timestamp so the bomb (index 0) wins
uint256 ts = _winningTimestamp(address(this), 0, 4, raffle.raffleStartTime() + duration);
vm.warp(ts);
vm.expectRevert("PuppyRaffle: Failed to send prize pool to winner");
raffle.selectWinner(); // permanently bricked
}

Recommended Mitigation

Use a pull-payment pattern: record the winner's prize and let them withdraw it, instead of pushing ETH inside selectWinner(). Update state and mint the NFT independently of the prize transfer.

mapping(address => uint256) public pendingPrize;
function selectWinner() external {
// ... pick winner, compute prizePool ...
previousWinner = winner;
pendingPrize[winner] += prizePool; // record instead of pushing
_safeMint(winner, tokenId); // never blocked by a reverting receiver
}
function claimPrize() external {
uint256 amount = pendingPrize[msg.sender];
pendingPrize[msg.sender] = 0;
(bool ok,) = msg.sender.call{value: amount}("");
require(ok, "PuppyRaffle: prize transfer failed");
}
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!