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

function getActivePlayerIndex() returns wrong number is player is not present

ETHANHUNTIMF99

Summary

The function returns 0 if the player is not found in the array which is not correct as in Solidity the array indices starts from zero that means the result will be wrong and the funds will be transferred to wrong participant.

Vulnerability Details

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

The first participant which have index 0 will be returned through this function.

Impact

The function getActivePlayerIndex is incorrect as it does not handle the case where the first player's address is found correctly. It returns 0 when the first player is found in the array, which may lead to a false positive result indicating that the player's index is 0.

In Solidity, array indices start from 0, and if the function returns 0 for a player that is found in the first position of the array, it can be misleading and incorrect.

Here's my proof of test code that shows that the function is wrongly implemented:

// test/PuppyRaffle.test.js
​
const { expect } = require("chai");
const { ethers } = require("hardhat");
​
describe("PuppyRaffle", function () {
it("getActivePlayerIndex should return the correct index for the first player", async function () {
const PuppyRaffle = await ethers.getContractFactory("PuppyRaffle");
const puppyRaffle = await PuppyRaffle.deploy();
await puppyRaffle.deployed();
​
// Add players to the contract
const players = ["0x1", "0x2", "0x3"];
await puppyRaffle.addPlayers(players);
​
// Call the getActivePlayerIndex function for the first player
const playerIndex = await puppyRaffle.getActivePlayerIndex("0x1");
​
// Assert that the playerIndex should not be 0
expect(playerIndex).to.not.equal(0);
});
});
​

Tools Used

Remix IDE, Hardhat

Recommendations

To make the function correct, one should consider returning a value that clearly indicates when the player is not found in the array. For example, you can return -1 or another suitable value to indicate that the player is not active or does not exist in the array.

- function getActivePlayerIndex(address player) external view returns (uint256) {
+ function getActivePlayerIndex(address player) external view returns (int256) {
​
- return 0;
+ return -1;
Updates

Lead Judging Commences

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