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

Smart Contract Wallet Incompatibility in selectWinner()

ravigaikar88

Root + Impact

Description

  • The selectWinner() function uses _safeMint() after sending ETH to the winner. The _safeMint() function calls onERC721Received() on the recipient if it's a contract. This creates two issues: 1) If the winner is a smart contract wallet that doesn't implement onERC721Received(), the transaction reverts and the raffle cannot complete, 2) The winner contract can intentionally revert in onERC721Received() to DoS the raffle if they don't like the NFT rarity they would receive.

// Root cause in the codebase with @> marks to highlight the relevant section
function selectWinner() external {
// ...
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
_safeMint(winner, tokenId); // Can revert for contract wallets
}

Risk

Likelihood:

  • The function performs the following steps:

    1. Sends ETH to the winner using a low-level call

    2. Mints an NFT to the winner using _safeMint


    The _safeMint call triggers onERC721Received if the recipient is a contract. A malicious contract can deliberately revert during this callback (e.g., based on NFT rarity), causing the entire selectWinner execution to revert.


    Since the raffle state is finalized inside this function, repeated reverts result in a Denial of Service, preventing the raffle from ever completing.

Impact:

  • The raffle can be permanently stuck if a smart contract wallet wins but doesn't implement ERC721Receiver. Additionally, malicious players can grief the protocol by reverting when they would receive common NFTs, forcing the raffle to remain unresolved.

Proof of Concept

A malicious contract can revert during NFT receipt:

contract MaliciousPlayer {
PuppyRaffle puppyRaffle;
function enterRaffle() external payable {
address;
players[0] = address(this);
puppyRaffle.enterRaffle{value: msg.value}(players);
}
// Revert if we don't like the NFT rarity
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external returns (bytes4) {
uint256 rarity = puppyRaffle.tokenIdToRarity(tokenId);
// Only accept legendary NFTs
if (rarity != puppyRaffle.LEGENDARY_RARITY()) {
revert("I only accept legendary NFTs!");
}
return this.onERC721Received.selector;
}
receive() external payable {}
}

Recommended Mitigation

Use a pull pattern where winners claim their prizes, or use regular _mint() instead of _safeMint():

mapping(address => uint256) public winnerPrizes;
​
function selectWinner() external {
winnerPrizes[winner] += prizePool;
_mint(winner, tokenId);
}
​
function claimPrize() external {
uint256 amount = winnerPrizes[msg.sender];
require(amount > 0, "No prize");
winnerPrizes[msg.sender] = 0;
payable(msg.sender).transfer(amount);
}
Updates

Lead Judging Commences

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