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

Two refunds create duplicate zero slots and permanently lock the raffle

phantombugz

Root + Impact

Description

refund() preserves the players array length and replaces refunded entries with address(0). After any two players refund, the array contains two identical zero-address placeholders. Every later enterRaffle() call appends the new participant and then rechecks all historical array pairs. The comparison between the two zero slots always fails the duplicate-player requirement.

At the same time, selectWinner() requires players.length >= 4. Starting from two refunded entries, the raffle has length two, cannot accept another entry, and cannot settle to clear the array. The contract is permanently stuck.

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);
​
// @> Multiple refunds create duplicate address(0) entries.
players[playerIndex] = address(0);
}
​
function enterRaffle(address[] memory newPlayers) public payable {
// ...
for (uint256 i = 0; i < players.length - 1; i++) {
for (uint256 j = i + 1; j < players.length; j++) {
// @> Two historical zero slots make every future entry revert.
require(players[i] != players[j], "PuppyRaffle: Duplicate player");
}
}
}
​
function selectWinner() external {
// @> The two-slot stuck state cannot be settled and cleared.
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
}

Risk

Likelihood: High

  • Any two ordinary participants can enter and invoke their documented refund right.

  • The failure is deterministic after the second refund and requires no privileged access.

Impact: Medium

  • All future entries revert because duplicate address(0) placeholders remain.

  • Settlement also remains unavailable, so there is no in-contract recovery and the raffle instance is permanently unusable.

Proof of Concept

function testTwoRefundsPermanentlyLockRaffle() public {
address[] memory entrants = new address[](2);
entrants[0] = address(1);
entrants[1] = address(2);
raffle.enterRaffle{value: 2 ether}(entrants);
​
vm.prank(address(1));
raffle.refund(0);
vm.prank(address(2));
raffle.refund(1);
​
address[] memory newcomer = new address[](1);
newcomer[0] = address(3);
vm.expectRevert("PuppyRaffle: Duplicate player");
raffle.enterRaffle{value: 1 ether}(newcomer);
​
vm.warp(block.timestamp + 1 days + 1);
vm.expectRevert("PuppyRaffle: Need at least 4 players");
raffle.selectWinner();
}

The focused Foundry reproduction passes.

Recommended Mitigation

Do not retain zero-address tombstones in the participant array. Remove refunded players with swap-and-pop, or track active membership separately and exclude inactive slots from duplicate checks and active-player counts.

- players[playerIndex] = address(0);
+ players[playerIndex] = players[players.length - 1];
+ players.pop();

Add regression tests for multiple refunds followed by a new entry and settlement.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 1 day ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-04] `PuppyRaffle::refund` replaces an index with address(0) which can cause the function `PuppyRaffle::selectWinner` to always revert

## Description `PuppyRaffle::refund` is supposed to refund a player and remove him from the current players. But instead, it replaces his index value with address(0) which is considered a valid value by solidity. This can cause a lot issues because the players array length is unchanged and address(0) is now considered a player. ## Vulnerability Details ```javascript players[playerIndex] = address(0); @> uint256 totalAmountCollected = players.length * entranceFee; (bool success,) = winner.call{value: prizePool}(""); require(success, "PuppyRaffle: Failed to send prize pool to winner"); _safeMint(winner, tokenId); ``` If a player refunds his position, the function `PuppyRaffle::selectWinner` will always revert. Because more than likely the following call will not work because the `prizePool` is based on a amount calculated by considering that that no player has refunded his position and exit the lottery. And it will try to send more tokens that what the contract has : ```javascript uint256 totalAmountCollected = players.length * entranceFee; uint256 prizePool = (totalAmountCollected * 80) / 100; (bool success,) = winner.call{value: prizePool}(""); require(success, "PuppyRaffle: Failed to send prize pool to winner"); ``` However, even if this calls passes for some reason (maby there are more native tokens that what the players have sent or because of the 80% ...). The call will thankfully still fail because of the following line is minting to the zero address is not allowed. ```javascript _safeMint(winner, tokenId); ``` ## Impact The lottery is stoped, any call to the function `PuppyRaffle::selectWinner`will revert. There is no actual loss of funds for users as they can always refund and get their tokens back. However, the protocol is shut down and will lose all it's customers. A core functionality is exposed. Impact is high ### Proof of concept To execute this test : forge test --mt testWinnerSelectionRevertsAfterExit -vvvv ```javascript function testWinnerSelectionRevertsAfterExit() public playersEntered { vm.warp(block.timestamp + duration + 1); vm.roll(block.number + 1); // There are four winners. Winner is last slot vm.prank(playerFour); puppyRaffle.refund(3); // reverts because out of Funds vm.expectRevert(); puppyRaffle.selectWinner(); vm.deal(address(puppyRaffle), 10 ether); vm.expectRevert("ERC721: mint to the zero address"); puppyRaffle.selectWinner(); } ``` ## Recommendations Delete the player index that has refunded. ```diff - players[playerIndex] = address(0); + players[playerIndex] = players[players.length - 1]; + players.pop() ```

Support

FAQs

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

Give us feedback!