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

Trusted Owner Oracle for Winner

osamaebaid

Root + Impact

Description

  • After the tournament ends, the owner calls setWinner with a country index to declare the winning team, enabling winners to withdraw based on their shares.

  • This sets the _setWinner flag, calculates winning shares, and finalizes the vault balance for payouts.

  • The issue is that the owner has full control to set any country as winner, regardless of the actual tournament outcome, allowing a malicious or compromised owner to choose a country where they or colluders hold all shares.

  • There is no verification, oracle, or decentralized mechanism to ensure the winner matches real-world results, making the owner an untrusted oracle.

// In setWinner - unrestricted owner choice
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
​
@> require(countryIndex < teams.length, "Invalid country index"); // @> Only bounds check; no verification of actual winner
​
if (_setWinner) {
revert WinnerAlreadySet();
}
​
winnerCountryId = countryIndex;
winner = teams[countryIndex];
​
_setWinner = true;
​
_getWinnerShares();
​
_setFinallizedVaultBalance();
​
emit WinnerSet (winner);
return winner;
​
}

Risk

Likelihood:

  • Owner becomes malicious, compromised, or negligent after the event ends.

Impact:

  • Owner sets winner to a country with their own or colluding shares, stealing the entire pot.

  • All legitimate users lose their deposits, leading to total funds loss and eroded trust.

Proof of Concept

  • Add testMaliciousOwnerTheft function to briVaultTest.t.sol

  • Run forge test --mt testMaliciousOwnerTheft

function testMaliciousOwnerTheft() public {
// 1. Ensure countries are set (required for setWinner)
vm.prank(owner);
briVault.setCountry(countries);
​
// 2. Give both users enough allowance
vm.prank(user1);
mockToken.approve(address(briVault), 2 ether);
​
vm.prank(user2);
mockToken.approve(address(briVault), 2 ether);
​
// 3. Honest user deposits 1 ether and joins country 0
vm.prank(user1);
briVault.deposit(1 ether, user1);
vm.prank(user1);
briVault.joinEvent(0);
​
// 4. Colluder deposits 1 ether and joins country 1
vm.prank(user2);
briVault.deposit(1 ether, user2);
vm.prank(user2);
briVault.joinEvent(1);
​
// 5. Advance time past event end
vm.warp(eventEndDate + 1 days);
​
// 6. Malicious owner sets country 1 (colluder's country) as winner
vm.prank(owner);
briVault.setWinner(1);
​
// 7. Colluder withdraws ~1.97 ether (both deposits minus fees)
uint256 colluderBefore = mockToken.balanceOf(user2);
vm.prank(user2);
briVault.withdraw();
uint256 colluderAfter = mockToken.balanceOf(user2);
​
// 8. Honest user tries to withdraw → correctly reverts with didNotWin()
vm.expectRevert("didNotWin()");
vm.prank(user1);
briVault.withdraw();
​
// 9. Final assertions: prove theft occurred
uint256 stolenAmount = colluderAfter - colluderBefore;
assertApproxEqAbs(
stolenAmount,
1.97 ether, // ~1 ether (own) + 1 ether (victim) - fees
0.01 ether,
"Colluder stole honest user's funds"
);
​
emit log_named_decimal_uint("Colluder stole", stolenAmount, 18);
}

Recommended Mitigation

  • Integrate Chainlink oracle for verifiable winner (simplified example)

  • Add oracle setup in constructor (keyHash, subId, etc.)

  • Replace owner-set with oracle-verified

​
+ import {VRFConsumerBaseV2} from "@chainlink/contracts/src/v0.8/vrf/VRFConsumerBaseV2.sol";
​
function setWinner(uint256 requestId, uint256[] memory randomWords) public {
- // Owner call
+ // Called by Chainlink callback or verified oracle
​
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
​
+ // Verify random or oracle data maps to real winner (e.g., randomWords[0] % teams.length)
+ uint256 countryIndex = randomWords[0] % teams.length; // Or real oracle feed
​
if (_setWinner) {
revert WinnerAlreadySet();
}
​
winnerCountryId = countryIndex;
winner = teams[countryIndex];
​
_setWinner = true;
​
_getWinnerShares();
​
_setFinallizedVaultBalance();
​
emit WinnerSet (winner);
}
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!