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

Blank spots in the players array lead to unwanted features

marcologonz

Summary

When a participant drops from the raffle via the PuppyRaffle::refund function, it leaves a blank spot in the array. This affects to the winner selection as explained in following sections.

Vulnerability Details

At the time of removing a participants of the PuppyRaffle::players array, it does not being handled in the proper way. It leaves a blank spot in the array and it does not reduce the length of the array.

In this way, the PuppyRaffle::enterRaffle and PuppyRaffle::refund workflow allows users to make the length of PuppyRaffle::players array all the big that they want. That is useful at the time of exploiting other vulnerabilities such as the weak RNG in the PuppyRaffle:selectWinner function. Moreover the length of that array is used to calculate the prizePool...

Impact

Markup:

  1. Leaving blank spots in the players array leads to:

    1. When a winner is selected it can result in the winner being the address(0) in which case the ether will dissapear.

  2. Allowing users to modify the length of PuppyRaffle::players at no cost leads to:

    1. More exploitability of the weak source of randomness in the PuppyRaffle:selectWinner function.

    2. The prizePool can be incremented as much as the attacker wants, as long as PuppyRaffle has sufficient funds.

Tools Used

Foundry

Recommendations

Handle participants' drop in a propper, and using an uint to keep track of the number of participants. For example:

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");
+ require(playerParticipating[playerAddress], "PuppyRaffle: Player already refunded, or is not active");
​
+ activePlayers--;
+ playerParticipating[playerAddress] = false;
​
payable(msg.sender).sendValue(entranceFee);
- players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}

If these changes are made, the PuppyRaffle code will have to be heavily refactored, which its necessary...

Updates

Lead Judging Commences

Hamiltonite Lead Judge about 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.

Give us feedback!