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

M-3: Smart contract winner without receive() permanently DoS-es selectWinner()

0xjjjjjj

Description

Severity: Medium

The enterRaffle() function at PuppyRaffle.sol:81-95 allows any address to enter, including smart contracts. The selectWinner() function at PuppyRaffle.sol:137 sends the prize pool to the winner via a low-level call:

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

If the winner is a smart contract that does not implement a receive() or fallback() function, the winner.call{value: prizePool}("") call will fail, causing selectWinner() to revert. Since selectWinner() is the only function that resets the players array, the raffle becomes permanently stuck.

Additionally, _safeMint at line 153 calls onERC721Received on the winner contract. If the contract does not implement IERC721Receiver, this also reverts.

Proof of Concept

contract NoReceive {
// Deliberately has no receive() or fallback()
PuppyRaffle target;
​
constructor(PuppyRaffle _target) {
target = _target;
}
​
function enter() external payable {
address[] memory players = new address[](1);
players[0] = address(this);
target.enterRaffle{value: msg.value}(players);
}
}
​
function testSmartContractWinnerReverts() public {
// Deploy contract without receive/fallback
NoReceive blocker = new NoReceive(puppyRaffle);
vm.deal(address(blocker), entranceFee);
blocker.enter();
​
// Enter 3 more players to meet minimum
address[] memory others = new address[](3);
others[0] = playerTwo;
others[1] = playerThree;
others[2] = playerFour;
puppyRaffle.enterRaffle{value: entranceFee * 3}(others);
​
vm.warp(block.timestamp + duration + 1);
​
// If blocker wins, selectWinner() reverts permanently
// because the contract cannot receive ETH
}

Risk

  • Impact: High — if the winning address is a contract without receive()/fallback(), selectWinner() permanently reverts. All player funds are locked with no recovery.

  • Likelihood: Medium — requires a smart contract entry to win. An attacker could deliberately enter with such a contract to grief the raffle.

Recommended Mitigation

Use a pull-over-push payment pattern. Instead of sending ETH directly in selectWinner(), record the winner and let them claim:

mapping(address => uint256) public pendingWithdrawals;
​
function selectWinner() external {
// ... winner selection logic ...
pendingWithdrawals[winner] += prizePool;
_safeMint(winner, tokenId);
}
​
function claimPrize() external {
uint256 amount = pendingWithdrawals[msg.sender];
require(amount > 0, "Nothing to claim");
pendingWithdrawals[msg.sender] = 0;
(bool success,) = msg.sender.call{value: amount}("");
require(success, "Transfer failed");
}

Alternatively, restrict raffle entry to EOAs only by checking msg.sender == tx.origin (note: this has its own tradeoffs and breaks smart wallet compatibility).

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 4 days 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!