BriVault

First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: low
Valid

No handling for "no winners" case → funds can become locked (and division-by-zero risk)

Root + Impact

Description

  • Normal behavior:
    After the tournament ends and the owner sets the winner, the vault should either allow winning participants to withdraw their proportional share or — if there are no winners for the selected team — safely refund participants or handle the pool deterministically.

  • Specific issue:

    setWinner() computes totalWinnerShares and sets finalizedVaultAsset, then withdraw() uses:uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);

    If the owner selects a winner that has no participants (i.e., totalWinnerShares == 0), two problems arise:

    • Logical lock: no-one can withdraw — participants of other teams are not winners and are denied; there is no fallback path (refunds) for the case where the winner bucket is empty, so funds become effectively unusable.

    • Division-by-zero: if due to some bug/edge-case (totalWinnerShares == 0) a withdraw() call reaches the Math.mulDiv(..., totalWinnerShares) path, it will revert (division by zero), causing more unexpected errors.

// Root cause in the codebase with @> marks to highlight the relevant section
@ uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);

Risk

Likelihood:

  • Reason 1: This occurs whenever the owner (maliciously or accidentally) sets the winner to a country index that has zero participants.

  • Reason 2: This occurs if participants forgot to join the actual winning team (e.g., mismatch between team names) or if totalWinnerShares was incorrectly aggregated to zero due to prior logic bugs (duplicate joins, incorrect bookkeeping).

Impact:

  • Funds can become effectively locked: honest participants who bet on other teams cannot withdraw because they "did not win", and there are no refunds or fallback.

If a legit winner exists but totalWinnerShares is zero due to aggregation bugs, legitimate winners cannot withdraw because the withdraw formula will revert.

  • High severity because all participants’ funds may be stuck with no on-chain recovery path.

Proof of Concept

User A deposits and joins country 0 (only participant).

  • Owner maliciously (or by mistake) calls setWinner(1) — country 1 has zero participants.

  • _getWinnerShares() yields totalWinnerShares == 0 and _setFinallizedVaultBalance() snapshots the vault balance.

  • No participant can call withdraw():

    • Non-winners (e.g., A) will revert with didNotWin() — they did not bet on the (incorrect) winner.

    • There are no winners to call withdraw() — even if someone attempts a withdraw (edge-case), division-by-zero or zero-division logic will revert.


// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
interface IBriToken { // minimal ERC20 used by vault
function approve(address spender, uint256 amount) external returns (bool);
function transfer(address to, uint256 amount) external returns (bool);
}
interface IBriVault {
function deposit(uint256 assets, address receiver) external returns (uint256);
function joinEvent(uint256 countryId) external;
function setWinner(uint256 countryIndex) external returns (string memory);
function withdraw() external;
}
contract NoWinnersPoC {
IBriVault public vault;
IBriToken public token;
constructor(address _vault, address _token) {
vault = IBriVault(_vault);
token = IBriToken(_token);
}
function run(address depositor, uint256 amount) external {
// Assume depositor has pre-approved vault to spend `amount`
// Step 1: depositor deposits and joins country 0
vault.deposit(amount, depositor);
// depositor calls joinEvent(0) (simulate by externally calling before)
// Step 2: owner sets winner to country 1 (no participants)
// For PoC we assume owner calls setWinner(1) off-chain; once done:
// Step 3: depositor cannot withdraw (didNotWin), and owner can't help.
}
}

Recommended Mitigation

Short explanation

Ensure setWinner() checks whether the chosen winner has participants (i.e., totalWinnerShares > 0) and provide a safe fallback for the "no winners" case — e.g., automatic refunds to all participants, allow owner to choose a different resolution, or burn fees and allow proportional refunds. Also guard withdraw() against division-by-zero by asserting totalWinnerShares > 0.

- remove this code
+ add this code
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];
- _setWinner = true;
-
- _getWinnerShares();
-
- _setFinallizedVaultBalance();
+ _setWinner = true;
+
+ _getWinnerShares();
+
+ // If no winners exist for selected country, revert and force owner to pick a valid winner.
+ // This prevents funds from being locked with zero denominator.
+ if (totalWinnerShares == 0) {
+ _setWinner = false;
+ totalWinnerShares = 0; // reset just in case
+ revert winnerNotSet(); // or a new error like NoWinnersForSelectedCountry()
+ }
+
+ _setFinallizedVaultBalance();
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Division by Zero in Withdraw Function When No Winners Bet on Winning Team

When no one bet on the winning team, making totalWinnerShares = 0, causing division by zero in withdraw and preventing any withdrawals.

Support

FAQs

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

Give us feedback!