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

Possibility for the organizer to be denied of withdrawing funds due to underflow.

enchev

Summary

It is possible for the organizer to be denied withdrawing the prediction fees.

Vulnerability Details

The method ThePredicter::withdrawPredictionFees is responsible for sending the prediction fees of participants to the organizer.
However, It is possible for the organizer to not be able to withdraw funds if enough players have withdrawn their prize pool depending on the set amount of fees for participation and registration.
The issue is that the following calculation uint256 fees = address(this).balance - players.length * entranceFee; can return negative value if address(this).balance is already very low due to players withdrawing their prize pools. This causes revert due to underflow.

Impact

The organizer cannot withdraw funds and they remain inaccessible in the contract.
Although the impact is high the likelyhood is low or medium at best.

Tools Used

Manual Review, Foundry

Proof Of Code

  1. Add the following test case to ThePredicter.test.sol

function test_organizerIsNotAbleToWithdrawFundsIfTooManyPlayersAlreadyDid() public {
vm.startPrank(organizer);
​
uint256 registrationFee = 0.04 ether;
uint256 predictionFee = 0.01 ether;
​
scoreBoard = new ScoreBoard();
thePredicter = new ThePredicter(
address(scoreBoard),
registrationFee,
predictionFee
);
scoreBoard.setThePredicter(address(thePredicter));
vm.stopPrank();
​
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
​
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: registrationFee}();
vm.stopPrank();
​
vm.startPrank(stranger2);
vm.deal(stranger2, 1 ether);
thePredicter.register{value: registrationFee}();
vm.stopPrank();
​
vm.startPrank(stranger3);
vm.deal(stranger3, 1 ether);
thePredicter.register{value: registrationFee}();
vm.stopPrank();
​
vm.startPrank(organizer);
thePredicter.approvePlayer(stranger);
thePredicter.approvePlayer(stranger2);
thePredicter.approvePlayer(stranger3);
vm.stopPrank();
​
address[3] memory players = [stranger, stranger2, stranger3];
​
// Let all players guess correctly 3 matches.
​
for (uint256 i = 0; i < players.length; i++) {
address player = players[i];
vm.startPrank(player);
​
thePredicter.makePrediction{value: predictionFee}(
0,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: predictionFee}(
1,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: predictionFee}(
2,
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();
​
for (uint256 i = 0; i < players.length; i++) {
address player = players[i];
​
vm.startPrank(player);
thePredicter.withdraw();
vm.stopPrank();
}
​
vm.startPrank(organizer);
vm.expectRevert();
thePredicter.withdrawPredictionFees();
vm.stopPrank();
}

  1. Execute the command forge test --mt test_organizerIsNotAbleToWithdrawFundsIfTooManyPlayersAlreadyDid -vvvvvvv

  2. From the logs of the command verify that ThePredicter::withdrawPredictionFee reverted and the reason for that is underflow - [Revert] panic: arithmetic underflow or overflow (0x11)

Recommendations

There are multiple mitigations for this issue:

  • Allow players to withdraw prize pool funds only after organizer has invoked the method ThePredicter::withdrawPredictionFee

  • Change the used formula in a way that it uses prediction fees to calculate the amount for transfering. Something like totalPredictionsCount * 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.