Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: medium
Valid

The Organizer cannot withdraw `PredictionFees` because an underflow occurs during the calculation of that value.

Summary

The Organizer should be able to withdraw the PredictionFee at any time, which is used for renting the venue. Consider a scenario where the PredictionFee value that the Organizer can withdraw is less than the funds allocated for player rewards. Additionally, assume that the players have withdrawn their due rewards. At this point, only the PredictionFee remains in the contract, which the Organizer should be able to withdraw.

However, if the Organizer attempts to withdraw the funds using the ThePredicter::withdrawPredictionFees function, a revert will occur due to the way the value the Organizer can withdraw is calculated.

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");
}

In the aforementioned scenario, address(this).balance is exactly the amount of the PredictionFee that the Organizer needs to withdraw, while players.length * entranceFee is a larger value. As a result, during the subtraction, an underflow occurs.

Impact

The Organizer cannot withdraw PredictionFees. The funds remain locked in the protocol.

PoC

function test_playersCollectRewardsGratterThenPredictionFeesThatOrganizerWillCollect() 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.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
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
);
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
);
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(stranger2);
thePredicter.withdraw();
vm.stopPrank();
assertEq(stranger2.balance, 0.9997 ether);
vm.startPrank(stranger3);
thePredicter.withdraw();
vm.stopPrank();
assertEq(stranger3.balance, 1.0397 ether);
vm.startPrank(organizer);
vm.expectRevert(); //we expect a revert due to the way the fee that can be collected by the Organizer is calculated
thePredicter.withdrawPredictionFees();
vm.stopPrank();
}

Recommendations

Change the mechanism for calculating the value of the fees variable in the ThePredicter::withdrawPredictionFees function, which represents the amount that the Organizer wants to and is entitled to withdraw.

Updates

Lead Judging Commences

NightHawK Lead Judge about 1 year 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.

Appeal created

elprofesor Submitter
about 1 year ago
NightHawK Lead Judge
about 1 year ago
NightHawK Lead Judge about 1 year 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.