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

`ScoreBoard::isEligibleForReward` does not follow the intended spec, leading to players not being eligible for prizes when they should

vile

Summary

ScoreBoard::isEligibleForReward does not follow the intended spec, leading to players who only submit one prediction to not receive rewards when they should.

Vulnerability Details

Currently, the isEligibleForReward function decides if a player is eligible for a reward if:

  1. All matches have a decided outcome (not Pending)

  2. The Player has a predictionsCount greater than one (non-inclusive, i.e. two or more)

This differs from the intended spec, as follows:

After the Organizer has entered the result from the last match (the 9th match), Players can take their rewards from the prize pool. Players can receive an amount from the prize fund only if their total number of points is a positive number and if they had paid at least one prediction fee.

As above, this means that predictionsCount should be compared inclusive, not exclusive.

Include the following test case in ./test/ThePredicter.t.sol:

function test_isEligibleForRewardDoesNotFollowSpec() public {
uint256 entranceFee = thePredicter.entranceFee();
uint256 predictionFee = 0.0001 ether;
​
// `stranger` makes 1 prediction (meets the threshold as described in the spec)
startHoax(stranger, 1 ether);
thePredicter.register{value: entranceFee}();
thePredicter.makePrediction{value: predictionFee}(0, ScoreBoard.Result.First);
vm.stopPrank();
​
// `organizer` sets prediction results
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();
​
// `stranger` is not eligible, even though they have "paid at least one prediction fee"
vm.prank(stranger);
vm.expectRevert(ThePredicter.ThePredicter__NotEligibleForWithdraw.selector);
thePredicter.withdraw();
}

Run the test:

forge test --mt test_isEligibleForRewardDoesNotFollowSpec -vvvvv

Impact

In certain circumstances, Players will be unable to claim prizes when they should be able to. This is unintended behavior.

Tools Used

Manual Analysis

Recommendations

Update the isEligibleForReward function to follow the intended spec:

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.