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

Division-by-Zero in withdraw() Leads to Permanent Freezing of All Vault Assets

3mr_obito4

Root + Impact

Description

  • The withdraw function calculates winnings by dividing the prize pool by the total shares of all winners. If the contract owner declares a winner that no participant chose, the number of winner shares becomes zero. This causes a division-by-zero error, making the withdraw function fail every time it's called. Because there is no other way to retrieve the funds, this error results in all assets being permanently locked in the vault.

Risk

Likelihood:

  • This scenario occurs whenever the declared winner is a team that no participant chose.

  • In events with many possible outcomes (e.g., a 48-team tournament) or low overall participation, it is plausible that an underdog winner might have zero backers, triggering the vulnerability.


Impact:

  • Permanent Loss of Funds: The division-by-zero error prevents the withdraw function from ever successfully executing for any user.

  • Total Contract Failure: As there is no alternative withdrawal or recovery mechanism, all assets within the vault become permanently frozen and cannot be retrieved by users or the contract owner.

Proof of Concept

The test demonstrates the vulnerability by having users deposit funds and bet on various teams. After the event, the owner intentionally declares a winner that nobody selected, which sets the count of "winner shares" to zero. The test then confirms that since no one is eligible to withdraw and the withdrawal function is now broken due to the division-by-zero error, all the deposited funds are proven to be permanently frozen.

function test_POC_LockFundsWhenNoWinner() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(4); // Bet on Brazil
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), 10 ether);
briVault.deposit(10 ether, user2);
briVault.joinEvent(10); // Bet on Japan
vm.stopPrank();
​
//
uint256 expectedVaultBalance = ((5 ether *
(10000 - participationFeeBsp)) / 10000) +
((10 ether * (10000 - participationFeeBsp)) / 10000);
assertEq(mockToken.balanceOf(address(briVault)), expectedVaultBalance);
console2.log(
"Total assets locked in vault before winner set:",
mockToken.balanceOf(address(briVault))
);
​
//
vm.warp(eventEndDate + 1 days);
vm.startPrank(owner);
briVault.setWinner(19); // Germany
vm.stopPrank();
​
//
assertEq(
briVault.totalWinnerShares(),
0,
"Total winner shares should be zero"
);
​
//
vm.startPrank(user1);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
briVault.withdraw();
vm.stopPrank();
​
vm.startPrank(user2);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
briVault.withdraw();
vm.stopPrank();
​
assertEq(
mockToken.balanceOf(address(briVault)),
expectedVaultBalance,
"Funds are still locked in the contract"
);
console2.log(
"Total assets permanently locked in vault after failed withdrawals:",
mockToken.balanceOf(address(briVault))
);
}

Recommended Mitigation

It is essential to handle the "no winner" case gracefully. This can be achieved by adding a check in the withdraw function and implementing a separate, owner-controlled function to manage the prize pool in this specific scenario.

error NoWinners();
​
function withdraw() external winnerSet {
if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
​
if (
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
keccak256(abi.encodePacked(winner))
) {
revert didNotWin();
}
+ if (totalWinnerShares == 0) {
+ revert NoWinners();
+ }
​
uint256 shares = balanceOf(msg.sender);
uint256 vaultAsset = finalizedVaultAsset;
uint256 assetToWithdraw = Math.mulDiv(
shares,
vaultAsset,
totalWinnerShares
);
// ... rest of function
}
​
+ /**
+ * @notice Allows the owner to recover all funds if no winner was found.
+ * @dev This function is a critical failsafe to prevent funds from being locked.
+ * It should only be callable after a winner has been set and totalWinnerShares is confirmed to be 0.
+ */
+ function recoverFundsIfNoWinner() external onlyOwner {
+ if (_setWinner != true) {
+ revert winnerNotSet();
+ }
+ if (totalWinnerShares != 0) {
+ revert("Cannot recover funds, there are winners");
+ }
+
+ uint256 totalBalance = IERC20(asset()).balanceOf(address(this));
+ IERC20(asset()).safeTransfer(owner(), totalBalance);
+ }
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!