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

withdraw() : Divide-by-zero / zero-denominator vulnerability

sidd

withdraw() — Divide-by-zero / zero-denominator vulnerability

Description

  • The withdraw() function computes the payment to winners using:

function withdraw() external winnerSet {
// some checks above
​
uint256 shares = balanceOf(msg.sender);
uint256 vaultAsset = finalizedVaultAsset;
​
@> // Vulnerability: totalWinnerShares can be 0 if no winners exist
@> // This causes a divide-by-zero revert in Math.mulDiv(...)
@> uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
​
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
​
// ...rest of function
}

If totalWinnerShares == 0, the operation attempts to divide by zero (the denominator of the ratio is zero). Depending on the Math.mulDiv implementation, this will revert (throw). In practice this leads to:

  • a runtime revert when winners attempt withdrawal, and

  • potential denial-of-service (winners cannot claim funds) or funds becoming effectively locked, depending on how the owner/admin and users react.

Multiple realistic contract states can make totalWinnerShares == 0 at withdraw() time:

  1. Owner calls setWinner(countryIndex) for a team that no participant joined (i.e., zero participants mapped to that winnerCountryId).

  2. Logic bugs upstream (e.g. joinEvent() incorrectly records shares, duplicates in usersAddress or userSharesToCountry but with zero stored shares) accidentally leave totalWinnerShares == 0.

  3. Interaction of the earlier deposit() bug (minting to msg.sender instead of receiver) can lead to userSharesToCountry entries being zero, so the sum over usersAddress yields 0.

  4. totalWinnerShares is incorrectly accumulated or stale across runs (the current _getWinnerShares() increments totalWinnerShares without resetting — this can also produce unexpected values, including zero if initial state is wrong).

Because withdraw() uses Math.mulDiv with totalWinnerShares as the denominator, any case where totalWinnerShares == 0 will revert.

Risk

Likelihood: (Medium)

  • withdraw() only applies to winners, but real-world operator mistakes such as selecting a team with no participants can trigger this condition.

  • User experience or logic errors (for example, users failing to join correctly or mismatched deposit() and joinEvent() logic) can result in totalWinnerShares being zero at settlement.

  • Tests currently assume non-empty winner groups, but in production an operator misclick, empty team, or race condition could realistically occur.

Impact: (High)

  • Winners (or intended recipients) will be blocked from claiming funds.

  • Funds in the vault (and potential yield earned on Aave) may remain frozen until the owner deploys an emergency/unlock mechanism or upgrades the contract.

  • The contract can be exploited as a denial-of-service vector by manipulating state so totalWinnerShares becomes zero, causing reverts on all withdraw() calls.

  • Combined with other issues (such as mismatched stakedAsset vs shares), this may result in confusion, disputes, or total loss of user funds.

Proof of Concept

Scenario A — owner sets a winner with zero participants

  1. No user joins team index 5.

  2. Owner calls setWinner(5). _getWinnerShares() iterates usersAddress and sums userSharesToCountry[user][5] — sum is 0. totalWinnerShares becomes 0.

  3. If any address (erroneously) satisfies the winner check (unlikely but plausible with logic bugs) and calls withdraw(), Math.mulDiv(shares, vaultAsset, 0) will revert causing the call to fail.

Scenario B — interaction with deposit() bug (mint to msg.sender)

  • Karen deposits on behalf of Sid, causing deposit() to mint shares to Karen while recording stakedAsset[Sid].

  • Sid joins the event, but since balanceOf(Sid) == 0, their userSharesToCountry entry becomes 0.

  • Owner sets Sid’s country as the winner → _getWinnerShares() sums and returns 0.

  • When Sid calls withdraw(), it reverts with a divide-by-zero error, locking funds.

function test_withdraw_divideByZero_withLogs() public {
vm.startPrank(owner);
briVault.setCountry(countries);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1); // shares minted to owner, not user1
vm.stopPrank();
​
vm.startPrank(user1);
briVault.joinEvent(10); // user1 joins with 0 shares
vm.stopPrank();
​
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(10); // totalWinnerShares = 0
vm.stopPrank();
​
// Logging state before withdraw
console.log("sharesUser1:", briVault.balanceOf(user1));
console.log("finalizedVaultAsset:", briVault.finalizedVaultAsset());
console.log("totalWinnerShares:", briVault.totalWinnerShares());
console.log("userSharesToCountry[user1][winnerCountryId]:", briVault.userSharesToCountry(user1, briVault.winnerCountryId()));
console.log("userToCountry[user1]:", briVault.userToCountry(user1));
console.log("winner:", briVault.getWinner());
console.log("=== DIVZERO-DEBUG-END ===");
​
// Expect division-by-zero revert
vm.startPrank(user1);
vm.expectRevert();
briVault.withdraw(); // panic: division or modulo by zero (0x12)
vm.stopPrank();
}

Observed Results:

sharesUser1: 0
finalizedVaultAsset: 4925000000000000000
totalWinnerShares: 0
userSharesToCountry[user1][winnerCountryId]: 0
userToCountry[user1]: Japan
winner: Japan


totalWinnerShares == 0 confirms the denominator in Math.mulDiv(shares, finalizedVaultAsset, totalWinnerShares)
is zero, causing a revert (panic 0x12) — effectively locking all withdrawals for that event.

Recommended Mitigations:

@@
error WinnerAlreadySet();
+ error noWinnerParticipants(); // new: explicit error for zero winner shares
error limiteExceede();
@@
- function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
+ function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
@@
- winnerCountryId = countryIndex;
- winner = teams[countryIndex];
-
- _setWinner = true;
-
- _getWinnerShares();
-
- _setFinallizedVaultBalance();
+ winnerCountryId = countryIndex;
+ winner = teams[countryIndex];
+
+ // compute total winner shares (no stale accumulation)
+ _getWinnerShares();
+
+ // require at least one winner share to avoid division-by-zero later
+ if (totalWinnerShares == 0) {
+ revert noWinnerParticipants();
+ }
+
+ // then finalize vault and mark winner set
+ _setFinallizedVaultBalance();
+ _setWinner = true;
emit WinnerSet(winner);
return winner;
}
@@
- function _getWinnerShares() internal returns (uint256) {
- for (uint256 i = 0; i < usersAddress.length; ++i) {
- address user = usersAddress[i];
- totalWinnerShares += userSharesToCountry[user][winnerCountryId];
- }
- return totalWinnerShares;
- }
+ function _getWinnerShares() internal returns (uint256) {
+ uint256 sum = 0;
+ for (uint256 i = 0; i < usersAddress.length; ++i) {
+ address user = usersAddress[i];
+ sum += userSharesToCountry[user][winnerCountryId];
+ }
+ totalWinnerShares = sum;
+ return totalWinnerShares;
+ }
@@
- uint256 vaultAsset = finalizedVaultAsset;
- uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
+ // defensive: ensure there are winner shares to avoid divide-by-zero
+ if (totalWinnerShares == 0) {
+ revert noWinnerParticipants();
+ }
+
+ uint256 vaultAsset = finalizedVaultAsset;
+ uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);

These changes ensure totalWinnerShares is recalculated cleanly and validated before any division occurs. By computing the value into a local accumulator and reverting if it equals zero, the contract prevents a divide-by-zero panic and ensures that a winner can only be finalized when participants exist. A defensive check in withdraw() adds an additional safeguard, reverting with a clear noWinnerParticipants() error instead of locking funds.

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!