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

[H-03] Lack of access control in `ThePredicter::makePrediction` allows anyone to make prediction causing loss of funds for non-players

monstitude

Summary

The ThePredicter::makePrediction function allows anyone to make a prediction. So if someone who's not a player makes a prediction, they cannot retrieve their funds back.

Vulnerability Details

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

function makePrediction(
uint256 matchNumber,
ScoreBoard.Result prediction
) public payable {
if (msg.value != predictionFee) {
revert ThePredicter__IncorrectPredictionFee();
}
​
if (block.timestamp > START_TIME + matchNumber * 68400 - 68400) {
revert ThePredicter__PredictionsAreClosed();
}
​
scoreBoard.confirmPredictionPayment(msg.sender, matchNumber);
scoreBoard.setPrediction(msg.sender, matchNumber, prediction);
}

If you notice in the function above, you can see that ThePredicter::makePrediction does not check if a user is a player before allowing to make a prediction. This allows anyone to make a prediction, breaking the desired logic.

Impact

When a user who's not a player makes a prediction, it will cause a loss of funds for that user. 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 testAnyoneCanMakePrediction.

function testAnyoneCanMakePrediction() public {
address user1 = makeAddr("user1");
vm.deal(stranger, 1 ether);
vm.prank(stranger);
thePredicter.register{value: 0.04 ether}();
​
vm.startPrank(organizer);
vm.warp(2);
thePredicter.approvePlayer(stranger);
vm.stopPrank();
​
vm.prank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(
0,
ScoreBoard.Result.First
);
​
vm.deal(user1, 1 ether);
vm.prank(user1);
thePredicter.makePrediction{value: 0.0001 ether}(
0,
ScoreBoard.Result.First
);
}

The test above proves that anyone can make a prediction even if they are not a player.

Tools Used

Manual Review, Foundry

Recommendations

Add a security check which ensures only players can make predictions.

function makePrediction(
uint256 matchNumber,
ScoreBoard.Result prediction
) public payable {
+ if (playersStatus[msg.sender] != Status.Approved) {
+ revert ThePredicter__UnauthorizedAccess();
+ }
​
...

After this code is added, it can be confirmed that a only a player can make a prediction. Add the following test and run forge test --mt testAnyoneCanMakePrediction.

error ThePredicter__UnauthorizedAccess();
​
function testAnyoneCanMakePrediction() public {
address user1 = makeAddr("user1");
vm.deal(stranger, 1 ether);
vm.prank(stranger);
thePredicter.register{value: 0.04 ether}();
​
vm.startPrank(organizer);
vm.warp(2);
thePredicter.approvePlayer(stranger);
vm.stopPrank();
​
vm.prank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(
0,
ScoreBoard.Result.First
);
​
vm.deal(user1, 1 ether);
vm.prank(user1);
vm.expectRevert(
abi.encodeWithSelector(ThePredicter__UnauthorizedAccess.selector)
);
thePredicter.makePrediction{value: 0.0001 ether}(
0,
ScoreBoard.Result.First
);
}
Updates

Lead Judging Commences

NightHawK Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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