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

`PuppyRaffle::getActivePlayerIndex` Incorrectly Returns 0 for Non-Active Players

krisrenzo

Summary

The PuppyRaffle::getActivePlayerIndex function currently returns 0 when a player is not found in the players array. However, 0 is a valid index in the players array, which can lead to confusion or incorrect behavior.

Vulnerability Details

The getActivePlayerIndex function returns 0 for non-active players, even though players[0] returns an active address when the raffle is in session.

function getActivePlayerIndex(address player) external view returns (uint256) {
for (uint256 i = 0; i < players.length; i++) {
if (players[i] == player) {
return i;
}
}
- return 0;
}

Proof of Concept

The provided test suite demonstrates the validity and severity of this vulnerability.

How to Run the Test

Requirements:

  • Install Foundry.

  • Clone the project codebase into your local workspace.

Step-by-step Guide to Run the Test:

  1. Ensure the above requirements are met.

  2. Copy the test below and add it to PuppyRaffleTest.t.sol tests under the getActivePlayerIndex section.

  3. Execute the following command in your terminal to run the test:

forge test --match-test "testReturnZeroForNonActivePlayerAndActivePlayers"

Code

function testReturnZeroForNonActivePlayerAndActivePlayers() public {
address[] memory players = new address[](2);
players[0] = playerOne;
players[1] = playerTwo;
puppyRaffle.enterRaffle{value: entranceFee * 2}(players);
​
/// @notice playerThree is not active and returns 0
assertEq(puppyRaffle.getActivePlayerIndex(playerThree), 0);
​
/// @notice playerOne is active and returns 0
assertEq(puppyRaffle.getActivePlayerIndex(playerOne), 0);
}

Impact

Implications:

  1. Poor UX: This can lead to a poor user experience as the protocol provides incorrect information.

  2. Potential Manipulation: Any function or system that relies on the integrity of the getActivePlayerIndex return value is compromised due to this vulnerability.

Exploit Scenario:

John requests the index of an address and assumes the player's activity status based on the return value. John makes a transaction based on that return value, but the result of his transaction is unexpected due to the incorrect assumption about the getActivePlayerIndex return value.

Tools Used

  • Foundry

Recommendations

Instead of returning zero, it is recommended to revert the transaction with an error message that notifies the caller that the address isn't active.

function getActivePlayerIndex(address player) external view returns (uint256) {
for (uint256 i = 0; i < players.length; i++) {
if (players[i] == player) {
return i;
}
}
+ revert("Player is not active!");
}
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!