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.
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.
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)
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.
## 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); } ```
The contest is live. Earn rewards by submitting a finding.
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsThe contest is complete and the rewards are being distributed.