Summary

1.Incomplete Access Control

2.Incorrect Time Limit

Vulnerability Details

1.In the ScoreBoard contract, the confirmPredictionPayment function only restricts the caller, without ensuring that the player must be an authorized player. Similarly, the setPrediction function does not sufficiently restrict the caller, which leaves a vulnerability for attackers.

https://github.com/Cyfrin/2024-07-the-predicter/blame/839bfa56fe0066e7f5610197a6b670c26a4c0879/src/ScoreBoard.sol#L54

https://github.com/Cyfrin/2024-07-the-predicter/blame/839bfa56fe0066e7f5610197a6b670c26a4c0879/src/ScoreBoard.sol#L61

One could deploy a contract to register as a player and then utilize other accounts to deploy numerous attack contracts (since we bypassed player registration, we can add attack contracts without restrictions and participate in predictions, ultimately sharing the rewards). These attack contracts do not need to register as players; instead, they exploit certain loopholes to achieve the same goal. By having a contract already registered as a player call the confirmPredictionPayment function to confirm other attack contracts, all these contracts can then call setPrediction,

and also withdraw fuction have no strict inspection of the msg.sender ,

thereby participating with only one registration and prediction fee, but effectively having multiple accounts competing and sharing rewards.

2.Confusion in time constraints. In the ScoreBoard contract's setPrediction function, the time restriction is incorrect with if (block.timestamp <= START_TIME + matchNumber * 68400 - 68400).The requirement specifies that predictions should close each night at 7:00 PM, but here it calculates based on the time of the first match plus (n-1) * 19 hours. It should be changed to if (block.timestamp <= START_TIME - 3600 + matchNumber * 86400 - 86400). The same correction applies to the makePrediction function in ThePredicter contract.

https://github.com/Cyfrin/2024-07-the-predicter/blob/839bfa56fe0066e7f5610197a6b670c26a4c0879/src/ThePredicter.sol#L93

Impact

1.Since we've bypassed player registration, we can deploy 90 attack contracts in three directions beyond the 30-player limit, allowing each direction to have 30 correct guesses every time. The withdraw function doesn't restrict the withdrawer to being a player, but we can determine eligibility through the isEligibleForReward function. Therefore, as long as we withdraw first as players, we can divide some rewards that should have belonged to these 30 people.

2.The requirement specifies that predictions must be closed at 7:00 PM each day, which is one hour before the match. However, according to the given conditions:

• The first match closes on the evening of the 15th at 8:00 PM.

• The second match closes on the 16th at 3:00 PM.

• The third match closes on the 17th at 10:00 AM.

• ...

This discrepancy leads to an incorrect deadline for predictions.

Tools Used

Visual Studio Code

Recommendations

1.In the setPrediction function of the ScoreBoard contract and the makePrediction function of the ThePredicter contract, change if (block.timestamp <= START_TIME + matchNumber * 68400 - 68400) to if (block.timestamp <= START_TIME - 3600 + matchNumber * 86400 - 86400).

2.To prevent bypassing user registration, there are two viable approaches:

1. The first is to add a require(player == thePredicter, "player illegality") condition in the confirmPredictionPayment function of the ScoreBoard contract. Currently, this function only restricts the caller and does not enforce that player must be an approved participant. This addition also prevents unauthorized calls to the setPrediction and withdraw functions by malicious contracts.

   function confirmPredictionPayment(

   address player,

   uint256 matchNumber

   ) public onlyThePredicter {

   require(player == thePredicter,"player illegality");

   playersPredictions[player].isPaid[matchNumber] = true;

   }

2. The second approach is to add a restriction in the withdraw function that allows withdrawals only for registered players. By adding require(playersStatus[msg.sender] == Status.Approved, "not player"), even if attacking contracts participate in predictions, they cannot withdraw funds. Reward distribution is based on various player parameters and remains unaffected by these measures.

   function withdraw() public {

   require(playersStatus[msg.sender] == Status.Approved,"not player");

   ​

   ......

   }