First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

players[] doesn't correctly update the players

wallebach

Summary

Removal of players using refund() method creates gaps inside the players[] array.

Vulnerability Details

The main problem is that nested loop performs check of duplicates as follows:

require(players[i] != players[j], "PuppyRaffle: Duplicate player");

So, if more than 1 player is removed enterRaffle() will revert and it'll not be possible to add new players. Here is a test:

function testCanEnterRaffleAndRemove() public {
address[] memory players = new address[](4);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
players[3] = playerFour;
​
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
​
vm.prank(playerTwo);
puppyRaffle.refund(1);
vm.prank(playerThree);
puppyRaffle.refund(2);
​
// actually there are two elements
assertFalse(puppyRaffle.getPlayersLength() == 2);
​
// however the length is not updated
assertEq(puppyRaffle.getPlayersLength(), 4);
​
address[] memory readdedPlayers = new address[](3);
readdedPlayers[0] = playerThree;
readdedPlayers[1] = address(11);
readdedPlayers[2] = address(12);
​
vm.expectRevert("PuppyRaffle: Duplicate player");
puppyRaffle.enterRaffle{value: entranceFee * 3}(readdedPlayers);
}

I created a helper getPlayersLength() as follows:

function getPlayersLength() public view returns (uint256) {
return players.length;
}

It's a view function and doesn't affect the smart contract, but could be useful for testing purposes.

Impact

High. The vulnerability can break game workflow.

Tools Used

Manual check.

Recommendations

Avoid loops. Especially nested loops. Instead use mapping(uint256 => address) for tracking players.

Updates

Lead Judging Commences

Hamiltonite Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

refund-doesnt-reduce-players-array-size-causing-protocol-to-freeze

zero address can win the raffle

Funds are locked to no one. If someone gets the refund issue, they also got this issue. IMPACT: High Likelihood: High

Support

FAQs

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