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

The method for calculating Prediction Fees to be withdrawn is ineffective

0x0noob

Description

ThePredicter::withdrawPredictionFees calculates fees using uint256 fees = address(this).balance - players.length * entranceFee; One probelm here is that it is possible for players.length * entranceFee; to be greater than the address(this).balance, and this can happens in different scenario over the lifespan of the project. One scenario would be say; when the organizer decides to collects fees when 31 players registers(assumption that 30 were approved, 1 is yet to collect back his registeration fee and that noone has made any prediction) and say the entranceFee is 0.04 ether. This would mean that the withdrawable Fee at this point is 0.04 ether * 31(address(this).balance) - (30 * 0.04); that is; 0.04 ether will be the only Fees that is withdrawable and the balance will be stucked in the contract.

A clearer POC for this is;

function test__withdrawalFeesGetStucked() public {
// 30 PLAYERS ENTERS
for (uint256 i = 0; i < 30; ++i) {
address user = makeAddr(string.concat("user", Strings.toString(i)));
vm.startPrank(user);
vm.deal(user, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(organizer);
thePredicter.approvePlayer(user);
vm.stopPrank();
}
// ADDITIONAL ONE PLAYERS ENTERS MAKING IT 31 PLAYERS
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.withdrawPredictionFees();
console.log(address(thePredicter).balance,1200000000000000000);
vm.stopPrank();
}

Another issue with the Fees is that it doesn't take into consideration whether the predictionFee has been paid by the players as the money in the protocol could just be from registration; implying that funds will be locked in the contract and that winners might not be able to get the reward of their predictions. A clearer explanation:

  1. 3 strangers registers (assumption that the registration fee is 0.04 ether this makes the balance of address(ThePredictor)to be 0.12 ether).

  2. Organizer approves 2 strangers implying that the players.length will by 2 and that uint256 fees = address(this).balance - players.length * entranceFee; which calculates the prediction fee to be withdrawn will be 0.12 ether - (2 * 0.04) which would be equal to 0.04 ether.
    This would mean that the organizer can withdraw 0.04 ether at this point even though no prediction has been made. This would also imply that if there all registrants are approved Fees will be 0implying that the function becomes useless.

Impact

Funds locked in contract, registrants not been able to retrieve their money back.

Tools Used

Manual

Recommendations

The calculation of Fees should not be based on players.length but there should be a way to track when prediction Fees is made.

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.