Puppy Raffle

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

Smart Contract wallet raffle winners without a `receive` or a `fallback` will block the start of a new contest

Smart Contract wallet raffle winners without a receive or a fallback will block the start of a new contest

Description

  • The PuppyRaffle::selectWinner function is responsible for resetting the lottery. However, if the winner is a smart contract wallet that rejects payment, the lottery would not be able to restart.

  • Non-smart contract wallet users could reenter, but it might cost them a lot of gas due to the duplicate check.

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: High

  • If the winner is a smart contract wallet that rejects payment, the lottery would not be able to restart.

Impact:

  • The PuppyRaffle::selectWinner function could revert many times, and make it very difficult to reset the lottery, preventing a new one from starting.

  • Also, true winners would not be able to get paid out, and someone else would win their money!

Proof of Concept

  1. 10 smart contract wallets enter the lottery without a fallback or receive function.

  2. The lottery ends

  3. The selectWinner function wouldn't work, even though the lottery is over!

Recommended Mitigation

There are a few options to mitigate this issue.

  1. Do not allow smart contract wallet entrants (not recommended)

  2. Create a mapping of addresses -> payout so winners can pull their funds out themselves, we could have a seperate claimPrize function letting the owners on the winner to claim their prize. (Recommended)

+ mapping(address user => uint256) public userPrize;
+ function claimPrize() external {
+ uint256 prize = userPrize[msg.sender];
+ (bool success,) = payable(msg.sender).call{value: prize}("");
+ require(success, "user claim prize fail!");
+ }

Pull Over Push pattern.

Updates

Lead Judging Commences

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