BriVault

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

Owner can arbitrarily set or modify the winner, enabling full fund theft

Root + Impact

If the owner acts maliciously or their private key is compromised, all funds in the vault can be stolen.
All users who bet on the “correct” team could lose their deposits to another team designated as the winner.
In practice, this risk makes the vault fully custodial, which defeats the purpose of using an on-chain betting vault.

Description

  • The vault relies on an owner account to finalize the tournament result by calling a function such as setWinner(teamId). However, there are no safeguards that limit when or how often this function can be called, or who verifies the correctness of the result.

  • If the owner acts maliciously or their private key is compromised, all funds in the vault can be stolen.

    All users who bet on the “correct” team could lose their deposits to another team designated as the winner.
    In practice, this risk makes the vault fully custodial, which defeats the purpose of using an on-chain betting vault.

function setWinner(uint256 teamId) external onlyOwner {
@> winner = teamId;
}

Risk

Likelihood:

  • The issue occurs whenever the tournament ends and the owner executes setWinner() to determine the winning team.

  • It will also occur whenever the owner’s private key is compromised or the owner acts dishonestly and selects an incorrect team to favor specific addresses.

Impact:

  • Impact 1: The entire pool of user funds can be redirected to a single team controlled by the owner or a malicious actor, resulting in a total loss for legitimate participants.

  • Impact 2: Users lose confidence in the fairness and integrity of the vault, making the system functionally custodial rather than decentralized, and potentially exposing deployers to legal and reputational damage.

Proof of Concept

users deposit:

  • Team A: 1000 tokens

  • Team B: 800 tokens

  • Real-world winner is Team A.

  • Owner calls setWinner(1) for Team B.

  • Users who bet on Team A cannot withdraw; Team B holders withdraw all 1800 tokens.


function setWinner(uint256 teamId) external onlyOwner {
winner = teamId;
}

Recommended Mitigation

This change:

  • Prevents multiple or arbitrary overwrites of the winner.

  • Restricts finalization to a fixed time window.

  • Emits an event for transparency.

  • Provides an optional emergency unlock path if the owner fails to act.

- function setWinner(uint256 teamId) external onlyOwner {
- winner = teamId;
- }
+ bool public winnerSet;
+ uint256 public winnerTeam;
+ uint256 public immutable finalizeDeadline;
+
+ event WinnerSet(uint256 indexed teamId, address indexed setter);
+
+ modifier beforeDeadline() {
+ require(block.timestamp <= finalizeDeadline, "Finalization deadline passed");
+ _;
+ }
+
+ function setWinner(uint256 teamId) external onlyOwner beforeDeadline {
+ require(!winnerSet, "Winner already set");
+ winnerSet = true;
+ winnerTeam = teamId;
+ emit WinnerSet(teamId, msg.sender);
+ }
+
+ // Optional: allow emergency unlock if no winner is set before the deadline
+ function emergencyUnlock() external {
+ require(block.timestamp > finalizeDeadline, "Too early for emergency unlock");
+ // Logic to allow proportional withdrawal or refund to all participants
+ }
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!