Submission Details
Severity:
Owner as Single Point of Failure
Summary
The ScoreBoard contract centralizes critical functions (setting match results and appointing the predicter) in the hands of a single owner, creating a significant risk of manipulation and abuse.
https://github.com/Cyfrin/2024-07-the-predicter/blob/main/src/ScoreBoard.sol#L46
https://github.com/Cyfrin/2024-07-the-predicter/blob/main/src/ScoreBoard.sol#L50
Vulnerability Details
The ScoreBoard contract designates the owner with exclusive privileges to set match results and appoint the predicter. This centralization of power poses a critical vulnerability as it enables the owner to manipulate the contract’s state and outcomes arbitrarily.
Impact
The owner can manipulate match results to favor specific players, giving them an unfair advantage. This undermines the integrity of the game and makes it impossible for other players to compete fairly. I have created POC in foundry to reflect the impact
// SPDX-License-Identifier: MIT
import "forge-std/Test.sol";
import "../src/ScoreBoard.sol";
contract ScoreBoardTest is Test {
ScoreBoard scoreboard;
address owner = address(1);
address predicter = address(2);
address player1 = address(3);
address player2 = address(4);
function setUp() public {
// Deploy the contract and set the owner
vm.prank(owner);
scoreboard = new ScoreBoard();
// Set the predicter
vm.prank(owner);
scoreboard.setThePredicter(predicter);
}
function testOwnerPrivilegeAbuse() public {
// Set initial predictions and payments
vm.prank(player1);
scoreboard.setPrediction(player1, 0, ScoreBoard.Result.First);
vm.prank(predicter);
scoreboard.confirmPredictionPayment(player1, 0);
vm.prank(player2);
scoreboard.setPrediction(player2, 0, ScoreBoard.Result.Second);
vm.prank(predicter);
scoreboard.confirmPredictionPayment(player2, 0);
// Owner sets the result in favor of player1
vm.prank(owner);
scoreboard.setResult(0, ScoreBoard.Result.First);
// Check the scores
int8 scorePlayer1 = scoreboard.getPlayerScore(player1);
int8 scorePlayer2 = scoreboard.getPlayerScore(player2);
assertEq(scorePlayer1, 2, "Player 1 should have score 2");
assertEq(scorePlayer2, -1, "Player 2 should have score -1");
}
}
Tools Used
Foundry, Manual Review
Recommendations
Use a multi-signature wallet for the owner address, requiring multiple parties to approve critical actions, reducing the risk of unilateral decisions.
// SPDX-License-Identifier: MIT
contract ScoreBoard {
uint256 private constant START_TIME = 1723752000; // Thu Aug 15 2024 20:00:00 GMT+0000
uint256 private constant NUM_MATCHES = 9;
enum Result {
Pending,
First,
Draw,
Second
}
struct PlayerPredictions {
Result[NUM_MATCHES] predictions;
bool[NUM_MATCHES] isPaid;
uint8 predictionsCount;
}
struct MultiSigApproval {
address[] approvals;
bool executed;
}
address[] public owners;
uint256 public requiredApprovals;
mapping(bytes32 => MultiSigApproval) public multiSigApprovals;
address thePredicter;
Result[NUM_MATCHES] private results;
mapping(address => PlayerPredictions) playersPredictions;
error ScoreBoard__UnauthorizedAccess();
error ScoreBoard__AlreadyExecuted();
error ScoreBoard__NotEnoughApprovals();
error ScoreBoard__InvalidOwner();
modifier onlyOwner() {
bool isOwner = false;
for (uint256 i = 0; i < owners.length; i++) {
if (owners[i] == msg.sender) {
isOwner = true;
break;
}
}
if (!isOwner) {
revert ScoreBoard__UnauthorizedAccess();
}
_;
}
modifier onlyThePredicter() {
if (msg.sender != thePredicter) {
revert ScoreBoard__UnauthorizedAccess();
}
_;
}
constructor(address[] memory _owners, uint256 _requiredApprovals) {
require(_owners.length >= _requiredApprovals, "Invalid number of owners or approvals");
owners = _owners;
requiredApprovals = _requiredApprovals;
}
function setThePredicter(address _thePredicter) public onlyOwner {
bytes32 txHash = keccak256(abi.encode("setThePredicter", _thePredicter));
require(!multiSigApprovals[txHash].executed, "Transaction already executed");
multiSigApprovals[txHash].approvals.push(msg.sender);
if (multiSigApprovals[txHash].approvals.length >= requiredApprovals) {
thePredicter = _thePredicter;
multiSigApprovals[txHash].executed = true;
}
}
function setResult(uint256 matchNumber, Result result) public onlyOwner {
bytes32 txHash = keccak256(abi.encode("setResult", matchNumber, result));
require(!multiSigApprovals[txHash].executed, "Transaction already executed");
multiSigApprovals[txHash].approvals.push(msg.sender);
if (multiSigApprovals[txHash].approvals.length >= requiredApprovals) {
results[matchNumber] = result;
multiSigApprovals[txHash].executed = true;
}
}
function confirmPredictionPayment(address player, uint256 matchNumber) public onlyThePredicter {
playersPredictions[player].isPaid[matchNumber] = true;
}
function setPrediction(address player, uint256 matchNumber, Result result) public {
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;
}
}
function clearPredictionsCount(address player) public onlyThePredicter {
playersPredictions[player].predictionsCount = 0;
}
function getPlayerScore(address player) public view returns (int8 score) {
for (uint256 i = 0; i < NUM_MATCHES; ++i) {
if (
playersPredictions[player].isPaid[i] &&
playersPredictions[player].predictions[i] != Result.Pending
) {
score += playersPredictions[player].predictions[i] == results[i]
? int8(2)
: -1;
}
}
}
function isEligibleForReward(address player) public view returns (bool) {
return
results[NUM_MATCHES - 1] != Result.Pending &&
playersPredictions[player].predictionsCount > 1;
}
}
''
Rewards breakdown
nSLOC
191This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.