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

The approved Players list in `ThePredicter` is not enforced which leads to miscalculated rewards

vile

Summary

ThePredicter does not properly guard functions that should only be used by Players, and not Users. This allows non-Players to particiapte without paying the registration fee, and leads to incorrect reward calculations.

Vulnerability Details

The management of the approved players list is implemented, but there is no logic to actually control what functions players and non-players can interact with (such as makePrediction).

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

function test_playersListIsNotEnforced() public {
uint256 predictionFee = 0.0001 ether;
​
// Assert the user is `Unknown` before making a prediction
// `Unknown` means the user has not (un)-registered previously
assertTrue(thePredicter.playersStatus(address(stranger)) == ThePredicter.Status.Unknown);
​
// `stranger` makes a predication without registering
hoax(stranger, predictionFee);
thePredicter.makePrediction{value: predictionFee}(0, ScoreBoard.Result.First);
​
// Assert the user is *still* `Unknown`
assertTrue(thePredicter.playersStatus(address(stranger)) == ThePredicter.Status.Unknown);
// Assert the Predicter contract has only received the `predictionFee`
assertEq(address(stranger).balance, 0 ether);
assertEq(address(thePredicter).balance, predictionFee);
}

Run the test:

forge test --mt test_playersListIsNotEnforced -vvvvv

Impact

Non-Players can participate in the game without paying the registration fee, which leads to incorrect reward calculations in ThePredicter::withdraw. This happens since the prize pool is inherently zero-sum, meaning that one Player's win is another Player's loss (in the case of a non-Draw across the board). Therefore, non-registered Users effectively claim shares that are intended for registered Player's.

Tools Used

Manual Review

Recommendations

Enforce the approved players list. There are multiple ways to do this, but one such way would be through the use of a modifier on functions that should be guarded, such as:

modifier onlyApprovedPlayer() {
if (playersStatus[msg.sender] != Status.Approved) {
revert ThePredicter__UnauthorizedAccess();
}
_;
}
function makePrediction(
uint256 matchNumber,
ScoreBoard.Result prediction
- ) public payable {
+ ) public payable onlyApprovedPlayer {
// ...
}
Updates

Lead Judging Commences

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

makePrediction lacks access control

makePrediction has no access controls and any unapproved user can make predictions causing an incorrect calculation and distribution of rewards.

Support

FAQs

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