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

Malicious Organizer Can Seize All Fees After Registration, Preventing Registered Addresses from Cancelling and Receiving Refunds.

EyanIngles

Summary

The organizer can maliciously or unexpectedly withdraw fees from the contract once any address has registered and paid a fee. The organizer can call the function each time a new address registers, without the need for approval. This behavior prevents unapproved addresses from canceling their registration and receiving a refund. The only way for an unapproved address to get their money back is to wait for predictions to be made, which allows the organizer to accumulate more funds while reducing the rewards available for approved players who participate in the predictions.

Vulnerability Details

Use the following test in ThePredicter.test.solfile.

We have 2 tests:

  1. Test testOrganizerTakesUnapprovedFees demonstrates that an address that has registered but not been approved can have their fees taken by the organizer before approval. In this example, another address player1 registers, providing enough funds for unApprovedPlayer1 to cancel their registration and reclaim their fees.

//Case for a registered address to cancel and claim fee back after another address registers
function testOrganizerTakesUnapprovedFees() public {
address unApprovedPLayer1 = makeAddr("unApprovedPLayer1");
address player1 = makeAddr("player1");
address player2 = makeAddr("player2");
address player3 = makeAddr("player3");
​
vm.startPrank(unApprovedPLayer1);
vm.deal(unApprovedPLayer1, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(organizer);
thePredicter.withdrawPredictionFees();
thePredicter.approvePlayer(player1);
vm.stopPrank();
​
vm.startPrank(player1);
vm.deal(player1, 1 ether);
thePredicter.register{value: 0.04 ether}();
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);
vm.stopPrank();
​
vm.startPrank(unApprovedPLayer1);
thePredicter.cancelRegistration();
vm.stopPrank();
}

  1. Test testFailOrganizerTakesUnapprovedFees demonstrates that an address that has registered but not been approved can have their fees taken by the organizer before approval. In this example, two addresses register but are not approved. They attempt to cancel and reclaim their funds, but the organizer has already withdrawn their fees without approving them. As a result, the pool lacks sufficient funds to refund the unapproved players, leaving them unable to participate in the event.

//Fail case for when a address(unapproved) tries to cancel regristration before players gamble
function testFailOrganizerTakesUnapprovedFees() public {
address unApprovedPLayer1 = makeAddr("unApprovedPLayer1");
address unApprovedPLayer2 = makeAddr("unApprovedPLayer2");
​
vm.startPrank(unApprovedPLayer1);
vm.deal(unApprovedPLayer1, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(organizer);
thePredicter.withdrawPredictionFees();
vm.stopPrank();
​
vm.startPrank(unApprovedPLayer2);
vm.deal(unApprovedPLayer2, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(organizer);
thePredicter.withdrawPredictionFees();
vm.stopPrank();
​
vm.startPrank(unApprovedPLayer1);
thePredicter.cancelRegistration();
vm.stopPrank();
}

Impact

The vulnerability demonstrated in testOrganizerTakesUnapprovedFees allows the organizer to withdraw fees from unapproved addresses immediately after registration. This behavior prevents unapproved players from canceling their registration and reclaiming their funds unless additional addresses register. Consequently, this can lead to a situation where unapproved players are unable to recover their fees promptly, while the organizer accumulates more funds. Furthermore, when approved players make predictions and pay the fee, the overall winnings can be impacted, reducing the rewards available and affecting the fairness and transparency of the contract.

Tools Used

Manual Review

Recommendations

To ensure the organizer only claims fees from approved players, we can introduce a new storage array called playersFeesToClaim. Approved player addresses will be pushed into this array by the ThePredicter::approvePlayer function. When the organizer claims the fees through the ThePredicter::withdrawPredictionFees function, it will only claim fees from the players in this array. After claiming the fees, the array will be cleared to ensure fees are only claimed once from each player.

//ThePredicter smart contract storage Mitigation
address public organizer;
address[] public players;
+ address[] public playersFeesToClaim;
uint256 public entranceFee;
uint256 public predictionFee;
ScoreBoard public scoreBoard;
mapping(address players => Status) public playersStatus;
//ThePredicter::approvePlayer function Mitigation
​
function approvePlayer(address player) public {
if (msg.sender != organizer) {
revert ThePredicter__UnauthorizedAccess();
}
if (players.length >= 30) {
revert ThePredicter__AllPlacesAreTaken();
}
if (playersStatus[player] == Status.Pending) {
playersStatus[player] = Status.Approved;
players.push(player);
+ playersFeesToClaim.push(player);
}
}
//ThePredicter::withdrawPredictionFees function Mitigations
​
function withdrawPredictionFees() public {
if (msg.sender != organizer) {
revert ThePredicter__NotEligibleForWithdraw();
}
- uint256 fees = address(this).balance - players.length * entranceFee;
+ if (playersFeesToClaim.length > 0) {
+ uint256 fees = playersFeesToClaim.length * entranceFee;
+ delete playersFeesToClaim;
(bool success,) = msg.sender.call{value: fees}("");
require(success, "Failed to withdraw");
+ }
}
Updates

Lead Judging Commences

NightHawK Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

EyanIngles Submitter
11 months ago
NightHawK Lead Judge
11 months ago
EyanIngles Submitter
11 months ago
NightHawK Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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