Submission Details
Severity:
zero(0) not accounted for in protocol design and in calculating reward shares
Summary
All the
Predictersfunds will get stuck in the protocol if at the end of the tournament , themaxScoreis zero(0).
Vulnerability Details
based on the design stated in the docs,
The prize fund is distributed in proportion to the points collected by all Players with a positive number of points. If all Players have a negative number of points, they will receive back the value of the entry fee.
this doesn't account for the event where the highest scorer scored zero(0). hence the withdraw function will revert for all players if this event happens.
Proof of Code
below is a foundry test to prove
// SPDX-License-Identifier: UNLICENSED
import {Test, console} from "forge-std/Test.sol";
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
import {ThePredicter} from "../src/ThePredicter.sol";
import {ScoreBoard} from "../src/ScoreBoard.sol";
contract ThePredicterTest is Test {
ThePredicter public thePredicter;
ScoreBoard public scoreBoard;
address public organizer = makeAddr("Ivan");
// address public malicious = makeAddr("malicious");
function setUp() public {
vm.startPrank(organizer);
scoreBoard = new ScoreBoard();
thePredicter = new ThePredicter(
address(scoreBoard),
0.04 ether,//0.04 ether
0.0001 ether
);
scoreBoard.setThePredicter(address(thePredicter));
vm.stopPrank();
}
function setUpPlayersRegistrationAndApproval() private{
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: 4e16}();
vm.stopPrank();
// approve players
vm.startPrank(organizer);
thePredicter.approvePlayer(user);
vm.stopPrank();
}
}
function test_FundLossDueZeroValueMaxScore() public{
setUpPlayersRegistrationAndApproval();
address winner_1 = thePredicter.players(0);
// let's test with 8 players
for(uint i = 0; i < 8; ++i){
address player = thePredicter.players(i);
vm.startPrank(player);
vm.deal(player, 1 ether);
thePredicter.makePrediction{value: 0.0001 ether}(0, ScoreBoard.Result.First);
thePredicter.makePrediction{value: 0.0001 ether}(1, ScoreBoard.Result.First);
thePredicter.makePrediction{value: 0.0001 ether}(2, ScoreBoard.Result.Draw);
thePredicter.makePrediction{value: 0.0001 ether}(3, ScoreBoard.Result.First);
thePredicter.makePrediction{value: 0.0001 ether}(4, ScoreBoard.Result.Second);
thePredicter.makePrediction{value: 0.0001 ether}(5, ScoreBoard.Result.Draw);
vm.stopPrank();
// let's reset player balance to zero to understand the result test
vm.deal(player, 0);
}
// organizer records match result
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.Second);
scoreBoard.setResult(4, ScoreBoard.Result.Draw);
scoreBoard.setResult(5, ScoreBoard.Result.First);
scoreBoard.setResult(8, ScoreBoard.Result.First);
vm.stopPrank();
// all predicters check match result and withdraw
for(uint i = 0; i < 8; ++i){
address player = thePredicter.players(i);
vm.startPrank(player);
vm.expectRevert();//no body could withdraw any funds
thePredicter.withdraw();
vm.stopPrank();
}
//nobody could withdraw funds
assertTrue(thePredicter.players(0).balance == thePredicter.players(1).balance &&
thePredicter.players(1).balance == thePredicter.players(2).balance &&
thePredicter.players(2).balance == thePredicter.players(3).balance &&
thePredicter.players(3).balance == thePredicter.players(4).balance &&
thePredicter.players(4).balance == thePredicter.players(5).balance &&
thePredicter.players(5).balance == thePredicter.players(6).balance &&
thePredicter.players(6).balance == thePredicter.players(7).balance &&
thePredicter.players(7).balance == 0 ether);
}
}
Impact
predictersfunds get stuck in the protocol and nobody will get refund
Tools Used
Foundry Test
Manual review
Documentation
Recommendations
Review the protocol design and set refund for all
predictersif no player scored more than zero(0). To effect this in thewithdrawfunction, do the following check instead;maxScore <= 0.
int8 maxScore = -1;
int256 totalPositivePoints = 0;
for (uint256 i = 0; i < players.length; ++i) {
int8 cScore = scoreBoard.getPlayerScore(players[i]);
if (cScore > maxScore) maxScore = cScore;
if (cScore > 0) totalPositivePoints += cScore;
}
if (maxScore > 0 && score <= 0) {
revert ThePredicter__NotEligibleForWithdraw();
}
uint256 shares = uint8(score);
uint256 totalShares = uint256(totalPositivePoints);
uint256 reward = 0;
- reward = maxScore < 0
? entranceFee
: (shares * players.length * entranceFee * 1e11) / totalShares * 1e11;
if (reward > 0) {
scoreBoard.clearPredictionsCount(msg.sender);
(bool success, ) = msg.sender.call{value: reward}("");
require(success, "Failed to withdraw");
}
int8 maxScore = -1;
int256 totalPositivePoints = 0;
for (uint256 i = 0; i < players.length; ++i) {
int8 cScore = scoreBoard.getPlayerScore(players[i]);
if (cScore > maxScore) maxScore = cScore;
if (cScore > 0) totalPositivePoints += cScore;
}
if (maxScore > 0 && score <= 0) {
revert ThePredicter__NotEligibleForWithdraw();
}
uint256 shares = uint8(score);
uint256 totalShares = uint256(totalPositivePoints);
uint256 reward = 0;
+ reward = maxScore <= 0
? entranceFee
: (shares * players.length * entranceFee ) / totalShares;
if (reward > 0) {
scoreBoard.clearPredictionsCount(msg.sender);
(bool success, ) = msg.sender.call{value: reward}("");
require(success, "Failed to withdraw");
}
Updates
Lead Judging Commences
NightHawK 10 months ago
Submission Judgement Published 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.
Rewards breakdown
nSLOC
191This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.