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

[H-02] Lack of access control in `ScoreBoard::setPrediction` allows anyone to change a prediction of any user

monstitude

Summary

The ScoreBoard::setPrediction allows a user to change their prediction after it has been set without repaying the prediction fee. However, lack of access control allows anyone to change the prediction of any user.

Vulnerability Details

Relevant link - https://github.com/Cyfrin/2024-07-the-predicter/blob/839bfa56fe0066e7f5610197a6b670c26a4c0879/src/ScoreBoard.sol#L61

function setPrediction(
@> address player,
uint256 matchNumber,
Result result
) public {
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;
}
}

In the function above, we can see that it has a player parameter which is supposed to be used by the ThePredicter contract to set a prediction for a user. However, since it does not have any access control, it allows anyone to set the prediction for any user.

Impact

A malicious user can change a user's prediction. This breaks the desired logic. It can be demonstrated with the following POC.

Proof of Concept

The impact is demonstrated with the following test, which can be executed with forge test --mt testAnyoneCanChangePrediction.

function testAnyoneCanChangePrediction() public {
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
thePredicter.makePrediction{value: 0.0001 ether}(
0,
ScoreBoard.Result.First
);
vm.stopPrank();
​
address attacker = makeAddr("attacker");
vm.prank(attacker);
scoreBoard.setPrediction(stranger, 0, ScoreBoard.Result.Second);
}

The test above proves that anyone can set a prediction for a user.

Tools Used

Manual Review, Foundry

Recommendations

Add a security check which ensures only the thePredicter contract can set anyone's prediction, while a player can only change their own prediction.

function setPrediction(
address player,
uint256 matchNumber,
Result result
) public {
+ if (msg.sender != thePredicter && msg.sender != player) {
+ revert ScoreBoard__UnauthorizedAccess();
+ }
​
...

After this code is added, it can be confirmed that a player can only change their prediction and everything else is working as expected. Add the following test and run forge test.

function testPlayerCanOnlyChangeTheirPrediction() public {
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
thePredicter.makePrediction{value: 0.0001 ether}(
0,
ScoreBoard.Result.First
);
vm.stopPrank();
​
address attacker = makeAddr("attacker");
vm.prank(attacker);
vm.expectRevert();
scoreBoard.setPrediction(stranger, 0, ScoreBoard.Result.Second);
}
Updates

Lead Judging Commences

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