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

The Method withdrawPredictionFees() reverts if some players withraw() their share of entrance fee first

0xrochimaru

Summary

It's most likely that a player can call withdraw() once the tournament is over independent of whether the organiser has called withdrawPredictionFees() or not. In case some players have already withdrawn their entrance fee share the organiser withdrawPredictionFees() will revert every single time. This amount will be stuck in the contract.

Vulnerability Details

The method withdrawPredictionFees() calculates the fees by subtracting the total entranceFee from the contract balance. It is possible that some players has already taken out their share of entranceFee. Which will lead to the organiser fee share getting stuck in the contract as the contract will have less Eth and the arithmetic will lead to revert with underflow.

function withdrawPredictionFees() public {
if (msg.sender != organizer) {
revert ThePredicter__NotEligibleForWithdraw();
}
​
uint256 fees = address(this).balance - players.length * entranceFee;
(bool success, ) = msg.sender.call{value: fees}("");
require(success, "Failed to withdraw");
}

Impact

This will lead to all of the prediction fees getting stuck in the contract which should've been claimed by the organiser.

POC

function test_organiserGetsLessFeeIfPlayerWithdrawFirst() public {
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(stranger2);
vm.deal(stranger2, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(stranger3);
vm.deal(stranger3, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(organizer);
thePredicter.approvePlayer(stranger);
thePredicter.approvePlayer(stranger2);
thePredicter.approvePlayer(stranger3);
vm.stopPrank();
​
vm.startPrank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
4,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
5,
ScoreBoard.Result.Draw
);
vm.stopPrank();
​
vm.startPrank(stranger2);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
4,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
5,
ScoreBoard.Result.Draw
);
vm.stopPrank();
​
vm.startPrank(stranger3);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
4,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
5,
ScoreBoard.Result.Draw
);
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(stranger);
thePredicter.withdraw();
assertEq(stranger.balance, 0.9795 ether);
​
vm.expectRevert();
​
vm.startPrank(organizer);
thePredicter.withdrawPredictionFees();
vm.stopPrank();
​
}

Tools Used

Manual Code Review

Recommendations

You can add a check whether the organiser has pulled his fees or not. Otherwise call withdrawPredictionFees() as soon as first player tries to withdraw it's amount and update the method code as below:

function withdrawPredictionFees() public {
if (msg.sender != organizer) {
revert ThePredicter__NotEligibleForWithdraw();
}
​
uint256 fees = address(this).balance - players.length * entranceFee;
(bool success, ) = organizer.call{value: fees}("");
require(success, "Failed to withdraw");
}
Updates

Lead Judging Commences

NightHawK Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Wrong computation in withdrawPredictionFees

withdrawPredictionFees incorrectly computes the value to be transferred to the organizer, which leads to pending players not being able to cancel their registration, approved players not being able to claim their rewards and other errors.

Support

FAQs

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