First Flight #20: The Predicter
First Flight #20
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Duplicate entry in ThePredicter::approvePlayer Function causing a player to register multiple times.

Caesar

Relevant GitHub Links

https://github.com/Cyfrin/2024-07-the-predicter/blob/main/src/ThePredicter.sol#L72-L83

Summary

  • Duplicate Entry: The function adds the player to the players array without checking if they are already present.

function approvePlayer(address player) public {
if (msg.sender != organizer) {
revert ThePredicter__UnauthorizedAccess();
}
if (players.length >= 30) {
revert ThePredicter__AllPlacesAreTaken();
}
if (playersStatus[player] == Status.Pending) {
playersStatus[player] = Status.Approved;
players.push(player);
}
}

Vulnerability Details

When a player with Status.Pending is approved, their status is updated to Status.Approved and they are added to the players array using players.push(player). However, there is no check to see if the player is already in the players array, leading to potential duplicate entries.

function approvePlayer(address player) public {
if (msg.sender != organizer) {
revert ThePredicter__UnauthorizedAccess();
}
if (players.length >= 30) {
revert ThePredicter__AllPlacesAreTaken();
}
if (playersStatus[player] == Status.Pending) {
playersStatus[player] = Status.Approved;
// Issue: Player is added without checking for duplicates
@> players.push(player);
}
}

Impact

Players can appear multiple times in the players array. This can cause incorrect handling of player-specific logic, such as scoring and rewards, leading to potential exploits where players might gain unfair advantages.

Tools Used

Manual

Recommendations

Add a loop to check if the player is already in the players array before adding them. If the player is found, the function reverts with an appropriate error.

function approvePlayer(address player) public {
if (msg.sender != organizer) {
revert ThePredicter__UnauthorizedAccess();
}
if (players.length >= 30) {
revert ThePredicter__AllPlacesAreTaken();
}
+ // Check if the player is already in the players array
+ for (uint256 i = 0; i < players.length; i++) {
+ if (players[i] == player) {
+ revert ThePredicter__CannotParticipateTwice();
+ }
+ }
if (playersStatus[player] == Status.Pending) {
playersStatus[player] = Status.Approved;
players.push(player);
}
}
Updates

Lead Judging Commences

NightHawK Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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