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

A winner contract that cannot receive ETH or ERC721 makes selectWinner revert and blocks the draw

ward0x

Root + Impact

Description

  • Drawing a winner should always succeed once the time has passed and enough players entered.

  • selectWinner pushes the prize with call and then uses _safeMint. If the chosen winner is a contract that cannot receive ETH, or that does not implement onERC721Received, the whole transaction reverts. Smart-contract wallets and other contracts are allowed to enter.

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

Attack / failure path:

  1. Several players enter through smart-contract wallets or contracts without a payable receive() (or without onERC721Received).

  2. selectWinner picks one of them; the call returns false (or _safeMint reverts in the ERC721 receiver check).

  3. The whole transaction reverts, players is not cleared and raffleStartTime is not reset.

  4. Callers must keep retrying with other msg.sender / timestamps hoping for a different index; if all or most entrants are such contracts the round can never finish and their ETH stays locked.

Risk

Likelihood:

  • Needs a contract player without receive / onERC721Received among the entrants. If many players are such contracts the draw fails repeatedly.

Impact:

  • The round cannot finish until a caller happens to get a different winner index; with enough such players the raffle is stuck and funds are locked, and new rounds cannot start.

Proof of Concept

All tests run in a test contract that deploys PuppyRaffle(1e18, address(99), 1 days) and uses the helpers _players(start,n) (distinct addresses) and _enter(p) (pays 1e18 * p.length). Full file: test/AuditPoC.t.sol, forge test --mc AuditPoC passes.

contract NoReceive {}
​
function test_M4_contractWinnerWithoutReceiveBlocksDraw() public {
address[] memory p = new address[](4);
for (uint256 i; i < 4; i++) p[i] = address(new NoReceive());
_enter(p);
vm.warp(block.timestamp + 1 days);
vm.expectRevert("PuppyRaffle: Failed to send prize pool to winner");
r.selectWinner();
}

Recommended Mitigation

Pay the winner with a pull pattern so a bad recipient 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;
+ _mint(winner, tokenId);
}
+
+function claimPrize() external {
+ uint256 amount = pendingPrize[msg.sender];
+ pendingPrize[msg.sender] = 0;
+ (bool ok,) = msg.sender.call{value: amount}("");
+ require(ok, "PuppyRaffle: claim failed");
+}
Updates

Lead Judging Commences

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