Submission Details
Severity:
getActivePlayerIndex() returns 0 for notAPlayer
Summary
The function PuppyRaffle::getActivePlayerIndex() responds with 0 for the status notAPlayer. Consequently, notAPlayer holds the same index as the initial player in the array. As a result, notAPlayer can participate in the Raffle without paying and potentially win if the PuppyRaffle::winnerIndex is 0.
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;
}
Impact
//A notAPlayer has the same index as the first player in the array
function testGetActivePlayerIndex() public {
// Enter a few players into the raffle
address[] memory players = new address[](4);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
players[3] = playerFour;
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
// Check if the function correctly returns the index of a specific player
assertEq(puppyRaffle.getActivePlayerIndex(playerOne), 0);
console.log("The playerOne index:", puppyRaffle.getActivePlayerIndex(playerOne));
assertEq(puppyRaffle.getActivePlayerIndex(playerTwo), 1);
assertEq(puppyRaffle.getActivePlayerIndex(playerThree), 2);
assertEq(puppyRaffle.getActivePlayerIndex(playerFour), 3);
// Check if the function returns 0 for a player who is not in the raffle
assertEq(puppyRaffle.getActivePlayerIndex(playerFive), 0);
console.log("The playerFive index:", puppyRaffle.getActivePlayerIndex(playerFive));
}
Logs:
The playerOne index: 0
The playerFive index: 0
[PASS] testGetActivePlayerIndexManyPlayers() (gas: 93540)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.95ms
Tools Used
Manual review.
Recommendations
It's recommended to consider reverting the function rather than returning a value of 0.
function getActivePlayerIndex(address player) external view returns (uint256) {
for (uint256 i = 0; i < players.length; i++) {
if (players[i] == player) {
return i;
}
}
- return 0;
+ revert("Player not found");
}
Updates
Lead Judging Commences
Hamiltonite almost 2 years ago
Submission Judgement Published Assigned finding tags:
getActivePlayerIndex can say a player is both entered at slot 0 and inactive
Rewards breakdown
nSLOC
143This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.