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

Players cannot withdraw funds when highest score is 0.

enchev

Summary

Funds remain stucked in the contract ThePredicter due to division by 0 when the max score

Vulnerability Details

The method ThePredicter::withdraw allows players to withdraw their part of the pool if eligable.
However, it does not handle well the case in which the max score of all players is 0. That's because the formula it uses when max score is 0 or larger, relies on the denominator (the max score) to not be 0. This causes division by zero and the method does not transfer any amount to the participants leaving the contract funds to remain stucked.

Impact

Funds remain stucked in the contract ThePredicter and players are not allowed to withdraw their prize pool.

Tools Used

Manual review, Foundry

Proof Of Code

Follow these steps:

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

function test_zeroMaxScore() public {
// Setup 3 players
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
​
uint256 registrationFee = 0.04 ether;
uint256 predictionFee = 0.0001 ether;
​
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 once (the first match) and incorrectly twice so everyone ends with zero score (+2 - 1 - 1 = 0) and predictions.
​
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); // Everyone guessed correctly (+2)
scoreBoard.setResult(1, ScoreBoard.Result.Second); // Everyone guessed wrong (-1)
scoreBoard.setResult(2, ScoreBoard.Result.Second); // Everyone guessed wrong. (-1)
scoreBoard.setResult(3, ScoreBoard.Result.Second); // Noone participated afterwards
scoreBoard.setResult(4, ScoreBoard.Result.Second);
scoreBoard.setResult(5, ScoreBoard.Result.Second);
scoreBoard.setResult(6, ScoreBoard.Result.Second);
scoreBoard.setResult(7, ScoreBoard.Result.Second);
scoreBoard.setResult(8, ScoreBoard.Result.Second);
vm.stopPrank();
​
for (uint256 i = 0; i < players.length; i++) {
address player = players[i];
​
vm.startPrank(player);
vm.expectRevert();
thePredicter.withdraw();
vm.stopPrank();
}
// Verify that everyone has score of 0:
console.log(scoreBoard.getPlayerScore(stranger));
console.log(scoreBoard.getPlayerScore(stranger2));
console.log(scoreBoard.getPlayerScore(stranger3));
​
// Verifiy that no participant was able to withdraw funds
assert(address(thePredicter).balance == (3 * registrationFee + (3 * 3) * predictionFee));
}

  1. Run the following command to execute the test case: forge test --mt test_zeroMaxScore -vvvvvvvv

  2. Verify that the test executed successfully (meaning that no funds were withdrawn) and that the culprit behind is division by 0 (contained in the logs as - [Revert] panic: division or modulo by zero (0x12))

Recommendations

Handle the max score equal to zero case the same way as it's done for negative numbers:

- reward = maxScore < 0
+reward = maxScore <= 0
? entranceFee
: (shares * players.length * entranceFee) / totalShares;
Updates

Lead Judging Commences

NightHawK Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Possible maxScore of zero is not accounted

The checks related to maxScore do not account possible maxScore of zero leading to stuck funds or a division by zero error.

Support

FAQs

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