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

setPrediction() should only allow approved players to set a prediction

0xziin

Summary

In ScoreBoard.sol, setPrediction() should only allow approved players to set a prediction according to the contest description. But this function let any user set a prediction.

Vulnerability Details

Any user can set a prediction even if he is not approved as a Player by the Organizer because of a lack of check.
https://github.com/Cyfrin/2024-07-the-predicter/blob/main/src/ScoreBoard.sol#L61-L75
There is no Modifier or other proceeding checking if the user is an approved Player.

function setPrediction(
address player,
uint256 matchNumber,
Result result
) public {
if (block.timestamp <= START_TIME + matchNumber * 68400 - 68400)
playersPredictions[player].predictions[matchNumber] = result;
playersPredictions[player].predictionsCount = 0;
for (uint256 i = 0; i < NUM_MATCHES; ++i) {
if (
playersPredictions[player].predictions[i] != Result.Pending &&
playersPredictions[player].isPaid[i]
) ++playersPredictions[player].predictionsCount;
}
}

Impact

A non authorized user can play the game. This is basically cheating.

Tools Used

Code review with VisualCode.

Recommendations

Mitigation :

Adding a Modifier onlyPlayer() that is going to check if the user is indeed authorized by the Organizer to play the game.
This can be done by checking if user is present in players[].

modifier onlyPlayer() {
bool present;
//Check if the user is present in the list of authorized players
for(uint256 i; i < players.length; i++) {
if (msg.sender == players[i]) {
present = true;
}
}
//If not -> revert
if (!present) {
revert ScoreBoard__UnauthorizedAccess();
}
_;
}


This method will function but it is gas consuming because of the way we keep track of the authorized players.
Instead of using a address[] public playersthe developer should use a Mapping, this way we won't have to browse the entire players table when checking if a user is authorized to play or not.

Updates

Lead Judging Commences

NightHawK Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

setPrediction lacks access control

setPrediction has no access control and allows manipulation to Players' predictions.

Support

FAQs

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