First Flight #20: The Predicter

First Flight #20

Beginner FriendlyFoundry

100 EXP

Submission Details

Severity: medium

Invalid

A player can register and be approved more than once. This leads to incorrect rewards computation and limits the amount of players to less than 30

Summary

It is possible for a player to register and be approved more than once. This will affect the rewards computation and the maximum amount of different players that are able to participate.

Vulnerability Details

If the following steps occur, a player can be approved more than once:

The user calls the ThePredicter.register function
The organizer approves the player
The user calls the ThePredicter.register function again
The organizer approves the player

Between the second and third step other players could register, making it less obvious that a player has been approved two or potentially more times.

The consequences of having the same player approved more than once is that the functions withdraw and approvePlayer make use of players.length to compute the amount of rewards and the amount of players that can participate respectively.

The following PoC base on existing tests shows how a user can be approved more than once, and shows how it impacts the rewards computation and the maximum amount of different players:

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 ThePredicterTest 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_multipleRegistrationLeadsToIncorrectRewardsDistribution() 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();

// double registration of stranger

vm.startPrank(stranger);

vm.deal(stranger, 1 ether);

thePredicter.register{value: 0.04 ether}();

vm.stopPrank();

vm.startPrank(organizer);

thePredicter.approvePlayer(stranger);

vm.stopPrank();

// make predictions

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.First

);

thePredicter.makePrediction{value: 0.0001 ether}(

3,

ScoreBoard.Result.First

);

vm.stopPrank();

vm.startPrank(stranger3);

thePredicter.makePrediction{value: 0.0001 ether}(

1,

ScoreBoard.Result.First

);

thePredicter.makePrediction{value: 0.0001 ether}(

2,

ScoreBoard.Result.First

);

thePredicter.makePrediction{value: 0.0001 ether}(

3,

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();

vm.startPrank(organizer);

thePredicter.withdrawPredictionFees();

vm.stopPrank();

vm.startPrank(stranger2);

thePredicter.withdraw();

vm.stopPrank();

vm.startPrank(stranger3);

thePredicter.withdraw();

vm.stopPrank();

// the amount received by the players that

// registered only once is not what was expected

assertNotEq(stranger2.balance, 0.9997 ether);

assertNotEq(stranger3.balance, 1.0397 ether);

assertNotEq(address(thePredicter).balance, 0 ether);

}

function test_multipleRegistrationLeadsToIncorrectAmountOfMaxPlayers() public {

vm.startPrank(stranger);

vm.deal(stranger, 1 ether);

thePredicter.register{value: 0.04 ether}();

vm.stopPrank();

vm.startPrank(organizer);

thePredicter.approvePlayer(stranger);

vm.stopPrank();

// double registration of stranger

vm.startPrank(stranger);

thePredicter.register{value: 0.04 ether}();

vm.stopPrank();

vm.startPrank(organizer);

thePredicter.approvePlayer(stranger);

vm.stopPrank();

// register another 28 players

for (uint256 i = 1; i <= 28; ++i) {

address user = makeAddr(string.concat("user", Strings.toString(i)));

vm.startPrank(user);

vm.deal(user, 1 ether);

thePredicter.register{value: 0.04 ether}();

vm.stopPrank();

vm.startPrank(organizer);

thePredicter.approvePlayer(user);

vm.stopPrank();

}

// So far 29 different players have been approved,

// but `players` has 30 entries, so the next approval will fail

address lastUser = makeAddr("lastUser");

vm.startPrank(lastUser);

vm.deal(lastUser, 1 ether);

thePredicter.register{value: 0.04 ether}();

vm.stopPrank();

vm.expectRevert();

vm.startPrank(organizer);

thePredicter.approvePlayer(lastUser);

vm.stopPrank();

}

}

Impact

Incorrect computation of rewards for players
Decrease the maximum amount of players that can participate

Tools Used

Foundry

Recommendations

Add a check in the ThePredicter.register function to ensure that the player address is not already included in the players array. Or alternatively, another solution could be to revert if the player status is not Unknown.

Optionally:

Add events to the changes of status of players to have more visibility of the state of the users of the system.

Updates

Lead Judging Commences

NightHawK Lead Judge about 1 year ago

Submission Judgement Published

Invalidated

Reason: Non-acceptable severity

Appeal created

artilugio0 Submitter

about 1 year ago

NightHawK Lead Judge

about 1 year ago

NightHawK Lead Judge about 1 year ago

Submission Judgement Published

Invalidated

Reason: Non-acceptable severity

Rewards breakdown

nSLOC

191

This contest has a flat reward structure with the following payouts:

High

100 EXP

Medium

20 EXP

Low

2 EXP

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

Support

FAQs

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