Summary
as stated on the documentation 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 but the latter are not the case because player can only withdraw after they make 2 prediction.
Vulnerability Details
this vulnerability happens because the function ScoreBoard::isEligibleForReward use playersPredictions[player].predictionsCount > 1 instead of playersPredictions[player].predictionsCount >= 1
function isEligibleForReward(address player) public view returns (bool) {
@> return results[NUM_MATCHES - 1] != Result.Pending && playersPredictions[player].predictionsCount > 1;
}
also there are error in the logic of setPrediction where it does not count for the first match predictionCount for the player.
function setPrediction(address player, uint256 matchNumber, Result result) public onlyThePredicter {
if (block.timestamp <= START_TIME + matchNumber * 68400 - 68400) {
playersPredictions[player].predictions[matchNumber] = result;
}
playersPredictions[player].predictionsCount = 0;
for (uint256 i = 0; i < NUM_MATCHES; ++i) {
@> if (playersPredictions[player].predictions[i] != Result.Pending && playersPredictions[player].isPaid[i]) {
++playersPredictions[player].predictionsCount;
}
}
}
The line above always return false for the first match because the Result are still not being set for the first match.
POC
add the following code to the ThePredicter.test.sol:
function test_POCWithdrawAfterOnlyMakeOnePrediction() public {
address player = makeAddr("player");
vm.startPrank(player);
vm.deal(player, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.prank(organizer);
thePredicter.approvePlayer(player);
vm.prank(player);
thePredicter.makePrediction{value: thePredicter.predictionFee()}(0, ScoreBoard.Result.Draw);
vm.startPrank(organizer);
scoreBoard.setResult(0, ScoreBoard.Result.Draw);
scoreBoard.setResult(1, ScoreBoard.Result.Draw);
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();
uint256 balanceBeforeWithdraw = player.balance;
vm.prank(player);
thePredicter.withdraw();
uint256 balanceAfterWithdraw = player.balance;
assert(balanceAfterWithdraw > balanceBeforeWithdraw);
}
then run the following forge test --mt test_POCWithdrawAfterOnlyMakeOnePrediction
the result should FAIL:
Failing tests:
Encountered 1 failing test in test/ThePredicter.test.sol:ThePredicterTest
[FAIL. Reason: ThePredicter__NotEligibleForWithdraw()] test_POCWithdrawAfterOnlyMakeOnePrediction() (gas: 229352
Impact
protocol not functioning like as stated in the documentation
Tools Used
foundry
Recommendations
correct the operator used 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;
}
and correct the logic for the setPrediction function:
function setPrediction(address player, uint256 matchNumber, Result result) public onlyThePredicter {
if (block.timestamp <= START_TIME + matchNumber * 68400 - 68400) {
playersPredictions[player].predictions[matchNumber] = result;
}
+ // checks if this is the first match
+ if (results[0] == Result.Pending) {
+ playersPredictions[player].predictionsCount = 1;
+ } else {
playersPredictions[player].predictionsCount = 0;
for (uint256 i = 0; i < NUM_MATCHES; ++i) {
if (playersPredictions[player].predictions[i] != Result.Pending && playersPredictions[player].isPaid[i])
{
++playersPredictions[player].predictionsCount;
}
}
+ }
}
then run again the command forge test --mt test_POCWithdrawAfterOnlyMakeOnePrediction the result should PASS:
Ran 1 test for test/ThePredicter.test.sol:ThePredicterTest
[PASS] test_POCWithdrawAfterOnlyMakeOnePrediction() (gas: 224776)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.07ms (270.66µs CPU time)
