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

Incorrect Fee Calculation Allows Organizer to Withdraw Unclaimed Entrance Fees

Summary

The ThePredicter::withdrawPredictionFees function incorrectly calculates the fees available for withdrawal, potentially allowing the organizer to withdraw unclaimed entrance fees from unapproved users.

Vulnerability Details

The current implementation calculates fees as:

uint256 fees = address(this).balance - players.length * entranceFee;

This calculation assumes all users in the contract are approved players, which may not be the case. Users can register without being approved, and the README file states that unapproved users should be able to cancel their registration and withdraw their entrance fees.

Impact

This vulnerability allows the organizer to withdraw more funds than intended, including unclaimed entrance fees from unapproved users. This violates the protocol's design as described in the README, where unapproved users should be able to reclaim their entrance fees. It could lead to financial losses for unapproved users who haven't yet canceled their registration.

PoC

function test_usersCantClaimEntranceFeeIfOrganiserHaveWithrawn() public {
address user = makeAddr("user");
vm.deal(user, 1 ether);
vm.startPrank(user);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
// Ensure that the entrance fees have been payed
assertEq(user.balance, 0.96 ether);
assertEq(address(thePredicter).balance, 0.04 ether);
vm.startPrank(organizer);
// Organiser withraw prediction fees
thePredicter.withdrawPredictionFees();
vm.stopPrank();
// Organiser will receive funds even if there is not prediction fees
assertEq(organizer.balance, 0.04 ether);
assertEq(address(thePredicter).balance, 0 ether);
vm.startPrank(user);
vm.expectRevert();
thePredicter.cancelRegistration();
vm.stopPrank();
}

Tools Used

Manual review, Foundry

Recommendations

Modify the fee calculation to only consider prediction fees:

- uint256 fees = address(this).balance - players.length * entranceFee;
+ uint256 fees = players.length * predictionFee;
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.

Support

FAQs

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