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

setWinner() can be called before validating if the winning team exists, risking empty or incorrect winner assignment

eze001

Root + Impact

Description

  • Normally, when the owner finalizes the tournament with setWinner(), the function should strictly ensure that the countryIndex corresponds to a valid, non-empty team name already set in teams[].

  • However, setWinner() only checks countryIndex < teams.length and does not verify whether teams[countryIndex] actually contains a valid team string. If the country was never initialized in setCountry(), winner may become an empty string, making winner-related logic unreliable and potentially breaking withdrawal conditions.

// Root cause in the codebase with @> marks to highlight the relevant section
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
​
@> require(countryIndex < teams.length, "Invalid country index");
if (_setWinner) {
revert WinnerAlreadySet();
}
​
@> winnerCountryId = countryIndex;
@> winner = teams[countryIndex]; // no check that this is non-empty
_setWinner = true;
​
_getWinnerShares();
_setFinallizedVaultBalance();
​
emit WinnerSet(winner);
return winner;
}
​

Risk

Likelihood:

  • Occurs whenever the owner mistakenly sets a winner index before initializing teams properly or when a given slot in teams remains unassigned (empty string).

  • Can also occur during testing or redeployment phases if setCountry() was never called before setWinner().

Impact:

  • Impact 1: winner may be empty, making every withdraw() revert since no userToCountry matches an empty string, locking all funds.

  • Impact 2: Off-chain or front-end systems displaying tournament results may show a blank or incorrect winner, undermining trust and functionality.

Proof of Concept

Observed Effect:
The contract emits WinnerSet(""), and later withdraw() calls will revert due to failed didNotWin() checks since no user’s userToCountry equals the empty string.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
contract PoC_SetEmptyWinner {
BriVault public vault;
​
constructor(BriVault _vault) { vault = _vault; }
​
function simulateEmptyWinner() external {
// Owner forgets to call setCountry()
vault.setWinner(5); // index exists but no team set, winner = ""
}
}
​

Recommended Mitigation

**Explanation: **Adding a simple non-empty string validation ensures the chosen winner is properly initialized and valid. This prevents assigning an empty winner and protects the payout flow from breaking.

function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
​
require(countryIndex < teams.length, "Invalid country index");
if (_setWinner) {
revert WinnerAlreadySet();
}
​
+ // Validate that the country at index is properly set
+ require(bytes(teams[countryIndex]).length != 0, "Winner country not initialized");
​
winnerCountryId = countryIndex;
winner = teams[countryIndex];
_setWinner = true;
​
_getWinnerShares();
_setFinallizedVaultBalance();
​
emit WinnerSet(winner);
return winner;
}
​
Updates

Appeal created

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

Support

FAQs

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

Give us feedback!