First Flight #20: The Predicter

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:

and this code in the ScoreBoard.sol

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.