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

The getActivePlayerIndex function incorrectly returns 0 for non-existent players, potentially leading to misidentification

Osora9

Summary

The getActivePlayerIndex function in the PuppyRaffle contract returns 0 when querying the index of a non-existent player. This can result in the erroneous identification of a non-existent player as the first player in the players array.

Vulnerability Details

In the PuppyRaffle contract, the getActivePlayerIndex function iterates through the players array searching for a matching address. If the function does not find a match, it defaults to returning 0. This is problematic because it does not differentiate between non-existent players and the first player in the array.

Here is the part of the function causing the issue:

function getActivePlayerIndex(address player) external view returns (uint256) {
for (uint256 i = 0; i < players.length; i++) {
//match pattern
}
}
//this is a problem
return 0;
}

Impact

If external systems or users rely on this function to identify or verify player participation, it may lead to false positives or unintended actions, such as false rewards, misallocation of resources, or other unexpected behaviors.

POC

  1. Enter two players into the raffle.

  2. Query the index of a non-existent player.

  3. The function will return 0, even though the non-existent player is not in the raffle.

function testFalsePlayerIndexIsEqualToFirstPlayer() public {
address[] memory playersList = new address[](2);
playersList[0] = playerOne;
playersList[1] = playerTwo;
address nonPlayer = address(3);
puppyRaffle.enterRaffle{value: entranceFee * 2}(playersList);
​
assertEq(
puppyRaffle.getActivePlayerIndex(nonPlayer),
puppyRaffle.getActivePlayerIndex(playerOne)
);
}

Tools Used

Foundry

Recommendations

To resolve this issue, the function can be modified to return an invalid index (such as -1) when no match is found, clearly indicating the player does not exist.

- return 0;
+ return -1;

This will ensure that the returned index is always within the bounds of the players array if the player exists, and clearly outside of it if they do not.

Updates

Lead Judging Commences

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

getActivePlayerIndex can say a player is both entered at slot 0 and inactive

Support

FAQs

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

Give us feedback!