BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: medium
Invalid

setWinner Access Control Surface Vulnerability

bugnetter

Root + Impact

Description

  • Normal Behavior: Tournament administrators declare winners after event completion to enable proportional payouts to participants.

  • Specific Issue: The setWinner() function is controlled exclusively by the owner with no backup mechanism, timelock, or dispute resolution. If the owner key is compromised, lost, or the owner becomes unavailable, all participant funds become permanently locked

  • Root Cause: The vulnerability stems from a single-owner dependency in tournament completion.

function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
@> // ... validation logic
@> _setWinner = true; // ❌ IRREVERSIBLE STATE CHANGE
@> // ... winner calculation and balance finalization
}
modifier winnerSet() {
@> if (_setWinner != true) {
@> revert winnerNotSet(); // ❌ BLOCKS ALL WITHDRAWALS
@> }
_;
}
function withdraw() external winnerSet {
// ... payout logic that depends on winner being set
}

Risk

Likelihood: Medium

  • This vulnerability has medium likelihood because while owner key compromise is a realistic attack vector and owner unavailability represents an operational risk, both require specific circumstances that are not highly probable in well-managed production systems.

Impact: High

  • Complete Fund Loss: All tournament assets permanently locked

  • Protocol Failure: Tournament resolution mechanism completely broken

  • User Harm: Legitimate winners cannot access rightfully earned payouts

  • Scalable Impact: Affects all participants regardless of tournament size

Proof of Concept

The POC demonstrates how the setWinner() function creates a critical single point of failure where owner key compromise permanently locks all participant funds. It shows that when the owner declares a winner, the _setWinner flag becomes irreversibly set to true, and if the owner key is lost or compromised afterward, all participants are permanently unable to withdraw their funds.

The test verifies that legitimate tournament winners cannot access their rightfully earned payouts when the owner becomes unavailable after winner declaration.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import "forge-std/Test.sol";
import {BriVault} from "../src/briVault.sol";
import {ERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
/**
* PoC: setWinner Access Control Surface Vulnerability
* Impact: Permanent fund lockup via single owner dependency
*
* Root Cause: setWinner() requires onlyOwner and creates irreversible _setWinner = true.
* If owner key compromised/lost/unavailable, all participant funds permanently locked
* since withdraw() requires winnerSet modifier which depends on setWinner() being called.
*/
contract MockERC20 is ERC20 {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}
​
contract SetWinnerAccessControlPoC is Test {
BriVault vault;
MockERC20 asset;
​
address owner = makeAddr("owner");
address attacker = makeAddr("attacker");
address participant1 = makeAddr("participant1");
address participant2 = makeAddr("participant2");
address feeRecipient = makeAddr("feeRecipient");
​
uint256 constant PARTICIPATION_FEE_BPS = 100; // 1%
uint256 constant MINIMUM_AMOUNT = 100 ether;
uint256 EVENT_START;
uint256 EVENT_END;
​
function setUp() public {
EVENT_START = block.timestamp + 1 days;
EVENT_END = EVENT_START + 7 days;
​
asset = new MockERC20("Mock Token", "MOCK");
​
vm.startPrank(owner);
vault = new BriVault(
IERC20(address(asset)),
PARTICIPATION_FEE_BPS,
EVENT_START,
feeRecipient,
MINIMUM_AMOUNT,
EVENT_END
);
​
string[48] memory countries;
countries[0] = "Brazil";
countries[1] = "Argentina";
vault.setCountry(countries);
vm.stopPrank();
​
asset.mint(participant1, 1000 ether);
asset.mint(participant2, 1000 ether);
​
vm.startPrank(participant1);
asset.approve(address(vault), type(uint256).max);
vm.stopPrank();
​
vm.startPrank(participant2);
asset.approve(address(vault), type(uint256).max);
vm.stopPrank();
}
​
/**
* CRITICAL VULNERABILITY: Owner key compromise leads to permanent fund lockup
*
* Attack Flow:
* 1. Participants deposit and join Brazil team
* 2. Attacker compromises owner key and declares Argentina as winner
* 3. Brazil participants cannot withdraw (did not win)
* 4. All funds permanently locked with no recovery mechanism
*/
function test_OwnerKeyCompromise_PermanentFundLockup() public {
console.log("=== OWNER KEY COMPROMISE ATTACK ===");
​
// Phase 1: Participants join tournament
vm.startPrank(participant1);
vm.warp(EVENT_START - 1 hours);
vault.deposit(200 ether, participant1);
vault.joinEvent(0); // Join Brazil
vm.stopPrank();
​
vm.startPrank(participant2);
vault.deposit(200 ether, participant2);
vault.joinEvent(0); // Join Brazil
vm.stopPrank();
​
// Phase 2: Attacker compromises owner and declares wrong winner
vm.warp(EVENT_END + 1 hours);
vm.prank(owner); // Attacker uses compromised owner key
vault.setWinner(1); // Argentina wins (wrong!)
​
assertTrue(vault._setWinner(), "Winner flag set by compromised owner");
assertEq(vault.winner(), "Argentina", "Wrong winner declared");
​
// Phase 3: Brazil participants cannot withdraw
vm.startPrank(participant1);
vm.expectRevert(BriVault.didNotWin.selector);
vault.withdraw(); // ❌ BLOCKED
vm.stopPrank();
​
vm.startPrank(participant2);
vm.expectRevert(BriVault.didNotWin.selector);
vault.withdraw(); // ❌ BLOCKED
vm.stopPrank();
​
// Phase 4: Funds permanently locked
uint256 lockedFunds = asset.balanceOf(address(vault));
assertGt(lockedFunds, 0, "All funds permanently locked");
assertEq(vault.winner(), "Argentina", "Wrong winner permanently set");
​
console.log("=== CRITICAL VULNERABILITY DEMONSTRATED ===");
console.log("- Owner key compromise leads to permanent fund lockup");
console.log("- No recovery mechanism exists");
console.log("- Tournament system completely broken");
}
​
/**
* CRITICAL VULNERABILITY: Owner unavailability leads to permanent fund lockup
*
* Attack Flow:
* 1. Participants deposit and join tournament
* 2. Tournament ends but owner becomes unavailable (key lost/incapacitated)
* 3. setWinner() never called, winnerSet modifier blocks all withdrawals
* 4. All funds permanently locked with no recovery mechanism
*/
function test_OwnerUnavailability_PermanentFundLockup() public {
console.log("=== OWNER UNAVAILABILITY ATTACK ===");
​
// Phase 1: Participants join tournament
vm.startPrank(participant1);
vm.warp(EVENT_START - 1 hours);
vault.deposit(200 ether, participant1);
vault.joinEvent(0); // Join Brazil
vm.stopPrank();
​
vm.startPrank(participant2);
vault.deposit(200 ether, participant2);
vault.joinEvent(0); // Join Brazil
vm.stopPrank();
​
// Phase 2: Tournament ends, owner becomes unavailable
vm.warp(EVENT_END + 1 hours);
// Owner loses private key - setWinner() never called
​
assertFalse(vault._setWinner(), "Winner never set due to owner unavailability");
​
// Phase 3: Participants cannot withdraw
vm.startPrank(participant1);
vm.expectRevert(BriVault.winnerNotSet.selector);
vault.withdraw(); // ❌ BLOCKED
vm.stopPrank();
​
vm.startPrank(participant2);
vm.expectRevert(BriVault.winnerNotSet.selector);
vault.withdraw(); // ❌ BLOCKED
vm.stopPrank();
​
// Phase 4: Funds permanently locked
uint256 lockedFunds = asset.balanceOf(address(vault));
assertGt(lockedFunds, 0, "All funds permanently locked");
​
// Even after extended time, funds remain locked
vm.warp(block.timestamp + 365 days);
assertFalse(vault._setWinner(), "Winner still not set after 1 year");
assertEq(asset.balanceOf(address(vault)), lockedFunds, "Funds still locked");
​
console.log("=== CRITICAL VULNERABILITY DEMONSTRATED ===");
console.log("- Owner unavailability leads to permanent fund lockup");
console.log("- No backup mechanism or recovery exists");
console.log("- Tournament system economically unviable");
}
}
​

Recommended Mitigation

Implement multisig with timelock and emergency recovery mechanisms to eliminate the single point of failure while maintaining tournament integrity.

- function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
+ function setWinner(uint256 countryIndex) public returns (string memory) {
+ require(msg.sender == multisigWallet, "Only multisig can declare winner");
// ... validation logic
_setWinner = true; // IRREVERSIBLE STATE CHANGE
// ... winner calculation and balance finalization
}
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

The winner is set by the owner

This is owner action and the owner is assumed to be trusted and to provide correct input arguments.

Support

FAQs

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

Give us feedback!