First Flight #20: The Predicter

Summary

The ThePredicter::withdraw function gives access to player(s) to withdraw their rewards. But the ThePredicter::withdraw does not implement a check to ensure that player(s) do not make more than one withdrawal. This exposes the protocol to malicious player(s) that can make withdrawal calls more than once.

Vulnerability Details

Root Cause
The setPrediction() in the ScoreBoard contract lacks proper access control, allowing unauthorized users to call it directly. above the test scenero, stranger was able to withraw multiple times after calling setprediction which in turn updates the state of isEligibleForreward() as withdraw function does not affect the status of playersPredictions[player].isPaid[matchNumber] = true;making stranger withdraws multiple 0.04ETH entranceFee. This manipulation allows users to increment their prediction count, making them eligible for rewards multiple times.

Impact

This would lead to more than one unauthorized withdrawal by player(s). This would lead to drainage of the protocols funds.

Tools Used

Manual Review

Recommendations

Access Control: The function can be written to give access control to the onlyOwner make withdraws and send rewards to players the are eligoble for rewards.
Proper CEI Pattern: Update state before making any external calls
Ensure Reward is Positive: Only proceed with the external call if the reward is positive
Division by Zero Handling: Ensure division by zero is handled properly
Non-negative Score: Require the player's score to be non-negative