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

Raffle gets stuck or takes long time when newPlayer length and msg.value is 0.

theirrationalone

Summary

PuppyRaffle misbehaves when we call enterRaffle with zero addresses into the newPlayers array. Since it passes the very first require statement, it adds zero new players into the players array. However, when it goes to sanitize the players array, it eventually gets stuck in a loop hell but not infinite. As we're using Solidity version 7 in PuppyRaffle, there is no exception for underflow and overflow.

Vulnerability Details

enterRaffle
// let's say, We have newPlayers.length = 0 and msg.value = 0...
function enterRaffle(address[] memory newPlayers) public payable {
// msg.value == entranceFee * newPlayers.length => 0 == uint * 0 => 0 == 0 ..... Check Passed ✅
require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle");
​
// i = 0 < newPlayers.length = 0 ... 👈 for loop execution completed therefore condition is falsy. Let's go to next statement...
for (uint256 i = 0; i < newPlayers.length; i++) {
players.push(newPlayers[i]);
}
​
// Check for duplicates
// here is something interesting happening ...
// players.length = 0...
// players.length - 1 => 0 - 1 -> players length is uint256 type.
// i = 0 and players.length - 1 = type(uint256).max
// So, for loop will iterate until i = type(uint256).max
for (uint256 i = 0; i < players.length - 1; i++) {
for (uint256 j = i + 1; j < players.length; j++) {
require(players[i] != players[j], "PuppyRaffle: Duplicate player");
}
}
emit RaffleEnter(newPlayers);
}

Impact

Our machines have to run that for loop 115792089237316195423570985008687907853269984665640564039457584007913129639935x times 😂.... and i don't know how many months it will take to complete only just for testing 😂.

It depends on blockchain whether blockchain makes an exit from a prolong for loop by crashing the raffle or completes for loop's all iterations.
In the end there's no gain only loss for all actors.... adversary, participant, raffle owner, anybody else who calls enterRaffle with an empty address array and 0 value.

Tools Used

Manual Review

Recommendations

We can employ a require check to restrict enterRaffle call with empty address array and 0 value.

My Mitigation
require(newPlayers.length > 0, "PuppyRafle: Must have players");
enterRaffle Fixed
function enterRaffle(address[] memory newPlayers) public payable {
require(newPlayers.length > 0, "PuppyRafle: Must have players");
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]);
}
​
// Check for duplicates
for (uint256 i = 0; i < players.length - 1; i++) {
for (uint256 j = i + 1; j < players.length; j++) {
require(players[i] != players[j], "PuppyRaffle: Duplicate player");
}
}
emit RaffleEnter(newPlayers);
}
Updates

Lead Judging Commences

Hamiltonite Lead Judge about 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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

Give us feedback!