Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Anyone can call `ScoreBoard::setPrediction`, without limitation, regardless of their user status

Summary

ScoreBoard::setPrediction function has no implemented access control checks for users making unlimited predictions, without paying any fee. Additionally, due to the function's public visibility, it could be called externally by anyone which poses a risk of unbounded growth and potentially - DoS by high gas costs for operations that iterate over the mapping or for the storage costs associated with it.

Vulnerability Details

ScoreBoard::setPrediction function lacks access control to hinder unlimited predictions. Malicious users could call ScoreBoard::setPrediction numerous times and set their predictions into the playersPredictions mapping. In such cases, theoretically, the playersStatus mapping could grow very large. While this won't cause an "overflow," it can lead to large state sizes and increased gas costs.

Iterating operations through the mapping, such as this in l.72-77::ScoreBoard.sol and ScoreBoard::getPlayerScore would have high gas costs.

Paste the following code in the test suite as a proof of concept:

function test_anyoneCanCallSetPredictionWithoutFee() public {
vm.startPrank(stranger);
vm.warp(1723750000-68401);
vm.deal(stranger, 1 ether);
scoreBoard.setPrediction(stranger, 0, ScoreBoard.Result.Draw);
assertEq(stranger.balance, 1 ether);
ScoreBoard.Result prediction = scoreBoard.getPrediction(stranger, 0);
assertEq(uint8(prediction), uint8(ScoreBoard.Result.Draw), "Prediction was not set correctly");
}

and this code in the ScoreBoard.sol

function getPrediction(address player, uint256 matchNumber) public view returns (Result) {
return playersPredictions[player].predictions[matchNumber];
}

Impact

Unlimited writings in the playersStatus mapping even without fee payments for function calls, which would limit the impact somewhat, could brick the functionality of several functions.

Tools Used

Manual review

Recommendations

Add onlyThePredicter modifier to ScoreBoard::setPrediction function, which with the added check to the ThePredicter::makePrediction function for approved status of the msg.sender(which is another security issue already reported) will ensure no malicous calls by implementing limits on the number of entries in the mapping to prevent it from growing uncontrollably.

Updates

Lead Judging Commences

NightHawK Lead Judge 11 months 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.