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

If a player has made only one prediction and it turns out to be correct, they still won't be able to withdraw the reward that is due for the correct prediction.

Summary

Imagine a scenario where a player has made only one prediction, and that prediction turns out to be correct. In that case, the user would be able to withdraw the reward for the correct prediction. But it's not all that simple. In the function ThePredicter::withdraw, which is used for withdrawing funds, there is a call to the function isEligibleForReward, which is defined as follows in ScoreBoard:

function isEligibleForReward(address player) public view returns (bool) {
return
results[NUM_MATCHES - 1] != Result.Pending &&
@> playersPredictions[player].predictionsCount > 1;
}

As we can see in the highlighted line, this function will return TRUE if the match is not pending and if predictionsCount (which represents the count of predictions made by the player) is met. Otherwise, if any condition is not satisfied, it will return FALSE.

In the ThePredicter::withdraw function, there is a check that will cause a revert if ScoreBoard::isEligibleForReward returns FALSE.

function withdraw() public {
if (!scoreBoard.isEligibleForReward(msg.sender)) {
revert ThePredicter__NotEligibleForWithdraw();
}
...

So, in a scenario where the user has made only one prediction and it turns out to be correct, the function ThePredicter::withdraw will always revert.

Impact

As a result of this bug, a user who has made only one prediction and it turns out to be correct will never be able to withdraw their reward!

PoC

function test_playerCanNotWithdrawRewardsIfHeMadeOnePredictionAndThatWasCorrect() public {
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.approvePlayer(stranger);
vm.stopPrank();
vm.startPrank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
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(stranger);
vm.expectRevert();
thePredicter.withdraw();
vm.stopPrank();
console.log(scoreBoard.getPlayerScore(stranger));
}

Recommendations

One possible solution to the given problem is to modify the ScoreBoard::isEligibleForReward function as follows:

function isEligibleForReward(address player) public view returns (bool) {
return
results[NUM_MATCHES - 1] != Result.Pending &&
- playersPredictions[player].predictionsCount > 1;
+ playersPredictions[player].predictionsCount >= 1;
}
Updates

Lead Judging Commences

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

The eligibility criteria is wrong

Players with only one prediction cannot withdraw.

Support

FAQs

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