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

Incorrect Eligibility Check for Reward in isEligibleForReward Function

Tomas0707

Summary

The isEligibleForReward function in the ScoreBoard.sol contract incorrectly checks the number of predictions made by a player to determine their eligibility for a reward. Currently, a player needs to have made more than one prediction to be eligible, which excludes players who have made only one prediction.

Vulnerability Details

The function isEligibleForReward checks if the number of predictions made by a player is greater than one to determine eligibility for a reward. However, if the intent is to reward players who have made at least one prediction, this check should be adjusted to reflect that.

https://github.com/Cyfrin/2024-07-the-predicter/blob/839bfa56fe0066e7f5610197a6b670c26a4c0879/src/ScoreBoard.sol#L94-L98

Proof of Concept

This is the test code of reward check.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import {Test, console} from "forge-std/Test.sol";
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
import {ThePredicter} from "../src/ThePredicter.sol";
import {ScoreBoard} from "../src/ScoreBoard.sol";
import {ThePredicterTest} from "./ThePredicter.test.sol";
​
contract CheckWithdrawTest is ThePredicterTest {
function testWithdraw() 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(organizer);
thePredicter.withdrawPredictionFees();
vm.stopPrank();
​
​
vm.expectRevert(
abi.encodeWithSelector(ThePredicter__NotEligibleForWithdraw.selector)
);
vm.startPrank(stranger);
thePredicter.withdraw();
emit log_named_int("stranger.score", scoreBoard.getPlayerScore(stranger));
vm.stopPrank();
}
}
​

To test this code:

  • Input this code to new test solidity file: test/CheckWithdraw.test.sol.

  • Then run this command:

    forge test --match-path test/CheckWithdraw.test.sol --match-test testWithdraw -vvvv

  • The result is:

    ├─ [1744] ThePredicter::withdraw()
    │ ├─ [993] ScoreBoard::isEligibleForReward(stranger: [0x49052147F5D97A723DEBdf07680fFFaDAd29A5dC]) [staticcall]
    │ │ └─ ← [Return] false
    │ └─ ← [Revert] ThePredicter__NotEligibleForWithdraw()
    ├─ [5788] ScoreBoard::getPlayerScore(stranger: [0x49052147F5D97A723DEBdf07680fFFaDAd29A5dC]) [staticcall]
    │ └─ ← [Return] 2
    ├─ emit log_named_int(key: "stranger.score", val: 2)

As you can see, users who made only one prediction can't get their rewards.

Impact

  • Players who have made only one prediction are incorrectly excluded from receiving rewards, potentially leading to unfairness if rewards are intended for any player who participates.

  • The current implementation may result in players who meet all other conditions but made only one prediction being unfairly excluded from rewards.

Tools Used

Manual code review

Recommendations

Modify the isEligibleForReward function to check if the number of predictions made by the player is greater than zero rather than one. This adjustment ensures that players who have made at least one prediction are considered for rewards.

function isEligibleForReward(address player) public view returns (bool) {
return
results[NUM_MATCHES - 1] != Result.Pending &&
playersPredictions[player].predictionsCount > 0;
}
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.