Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

Refunding more than one players cause no new players to enter after

Summary

The function PuppyRaffle#refund() reset the address of the player refunded.
If two players get refunded, any subsequent call to the function PuppyRaffle#enterRaffle() will revert caused by the check on the duplicates and no new players will enter the raffle.

Vulnerability Details

The following line in PuppyRaffle#enterRaffle() is checking for equality but it is not taking address 0 into account.

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

Having more than one address 0 in the players array is caused by the PuppyRaffle#refund() function at this particular line.

players[playerIndex] = address(0);

A proof of concept of the attack is provided below.

function testTwoRefundCauseNoNewPlayers() public playersEntered {
vm.prank(playerOne);
puppyRaffle.refund(0);
vm.prank(playerTwo);
puppyRaffle.refund(1);
address playerFive = address(5);
vm.deal(playerFive, 1 ether);
address[] memory players = new address[](1);
players[0] = playerFive;
vm.expectRevert("PuppyRaffle: Duplicate player");
puppyRaffle.enterRaffle{value: entranceFee * 1}(players);
}

Impact

The previous situation can be caused either by a malicious user who requests a refund with 2 of his addresses or simply by two different players.

Tools Used

Manual review.

Recommendations

In the PuppyRaffle#enterRaffle() function make sure to take in consideration the address 0 when you are searching for duplicates.

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.