Puppy Raffle

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

### Smart contract raffle winners without a `fallback` or `receive` function cause `PuppyRaffle::selectWinner` to revert, discarding the legitimate winner

[M-1] Smart contract raffle winners without a fallback or receive function cause PuppyRaffle::selectWinner to revert, discarding the legitimate winner

Description: The PuppyRaffle::selectWinner function is responsible for resetting the lottery. If the low-level call ((bool success,) = winner.call{value: prizePool}("");) does not succeed, success would be false and revert in the next require line. Same issue happens with _safeMint if the recipient does not implement IERC721Receiver. Due to that, the winner of that raffle would be discarded.

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

Impact: If the winner turns out to be a smart contract without any fallback or receive function, the transaction will revert causing the selected winner to be discarded silently.
Same issue applies to _safeMint, which calls onERC721Received on the recipient. A contract that accepts ETH but does not implement IERC721Receiver will not accept the NFT and revert the whole transaction resulting in the same consequences as before. Either failure point discards the winner.

Proof of Concept:

  1. Four users (or more) enter the raffle, one of them being a smart contract wallet without any fallback or receive function.

  2. The raffle ends.

  3. PuppyRaffle::selectWinner fires and that one smart contract wallet wins. The contract tries to send the prize to the winner but, as it does not have any fallback or receive function, the call reverts and the winner gets nothing.

  4. Now the next call of PuppyRaffle::selectWinner function might select a different winner.

Note: Same issue applies to a smart contract wallet that does not implement IERC721Receiver, whole transaction will revert discarding the winner in the process.

Recommended Mitigation: There are a few mitigations to apply:

  1. Create a mapping of address -> payout amount so winners can claim their payout whenever they want through a new function designed to meet that need. Claiming must also cover the NFT, not just the ETH. Either mint with _mint instead of _safeMint, or have the winner claim the NFT through the same pull mechanism.

  2. Do not allow smart contract wallets to enter the raffle (not recommended: breaks legitimate smart contract wallets from real users).

Updates

Lead Judging Commences

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