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

Logic error - Players cannot withdraw if they made one prediction

JJS

Summary

The documentation clearly states that a player has to make at least one prediction to be able to withdraw their winnings. However, in the ScoreBoard::isEligibleForReward function, which decides if a player is eligible to withdraw, it is hard coded in the the prediction count has to be more than one. In the ThePreidcter::withdraw function, the first check is to see if a player is eligible to withdraw and uses that function to determine that.

Vulnerability Details

function withdraw() public {
if (!scoreBoard.isEligibleForReward(msg.sender)) {
@> revert ThePredicter__NotEligibleForWithdraw();
}
.
.
.
}
​
function isEligibleForReward(address player) public view returns (bool) {
@> return results[NUM_MATCHES - 1] != Result.Pending && playersPredictions[player].predictionsCount > 1;
}

Impact

This causes players that have got their score positive, but only made one prediction, to now be able to withdraw their funds. If that was the intention it has not be properly explained in the documentation.

Tools Used

Manual review and test

Here is a test that can be implemented in the ThePredicter.test.sol:

Step by step:

  1. A stranger registers.

  2. The organiser approves them.

  3. The stranger makes a single prediction.

  4. The organiser sets the results, finishing the games, and withdraw the fees for the hall.

  5. A stranger tries to withdraw but cannot.

function test_cannotWithdrawWithAtLeastOnePrediction() public {
vm.deal(stranger, 1 ether);
vm.prank(stranger);
thePredicter.register{value: 0.04 ether}();
​
vm.prank(organizer);
thePredicter.approvePlayer(stranger);
​
vm.prank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(1, ScoreBoard.Result.First);
​
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.prank(organizer);
thePredicter.withdrawPredictionFees();
​
vm.prank(stranger);
vm.expectRevert();
thePredicter.withdraw();
}

Recommendations

Either change the logic to mean more than zero or more and equal to one in the `isEligibleForReward` function:

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

Lead Judging Commences

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