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

No winning bets causes permanent lock of all funds

ynyesto

Description

The contract allows the owner to set any country as the winner, even if no users bet on that country (which of course is correct, since an underdog could win). When `setWinner()` is called, `_getWinnerShares()` calculates `totalWinnerShares` by summing shares for all users who bet on the winning country. If no user bet on the winning country, `totalWinnerShares` will be 0.

When no users bet on the winning country, all users will fail the check in `withdraw()` that verifies their country matches the winner, causing them to revert with `didNotWin()`. Since no one can withdraw, all assets remain permanently locked in the vault. Users still have their shares, but they cannot use them because they didn't bet on the winning country. There is no recovery mechanism to handle this scenario.


  • function _getWinnerShares () internal returns (uint256) {
    for (uint256 i = 0; i < usersAddress.length; ++i){
    address user = usersAddress[i];
    totalWinnerShares += userSharesToCountry[user][winnerCountryId]; // @> All entries = 0 if no one bet on winner
    }
    return totalWinnerShares; // @> Returns 0
    }
    ​
    function withdraw() external winnerSet {
    // ... validation ...
    if (
    keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
    keccak256(abi.encodePacked(winner))
    ) {
    revert didNotWin(); // @> All users revert here if no one bet on winner
    }
    // ... rest never executes ...
    }

Risk

**Likelihood**:
* Owner can set any country as winner, including ones no one bet on, since any country can win
​
**Impact**:
* Permanent lock of all assets in the vault
* All users lose their deposits with no way to recover
* Users' shares become worthless (cannot be withdrawn)
* No recovery mechanism exists

Proof of Concept

The PoC test below shows how if no one wins the bet funds are permanently frozen in the bet contract.

function testLossOfFundsIfNoUsersBetOnWinner() public {
vm.prank(owner);
briVault.setCountry(countries);
​
// Users bet on countries 10 and 20
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(10); // Bet on country 10
vm.stopPrank();
​
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
briVault.joinEvent(20); // Bet on country 20
vm.stopPrank();
​
uint256 vaultBalanceBefore = mockToken.balanceOf(address(briVault));
uint256 user1Shares = briVault.balanceOf(user1);
uint256 user2Shares = briVault.balanceOf(user2);
​
// Owner sets country 5 as winner (no one bet on it)
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(5);
uint256 totalWinnerShares = briVault.totalWinnerShares();
vm.stopPrank();
​
// Verify no winner shares
assertEq(totalWinnerShares, 0);
​
// No one can withdraw - all get didNotWin()
vm.prank(user1);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
briVault.withdraw();
​
vm.prank(user2);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
briVault.withdraw();
​
// Assets are locked forever
uint256 vaultBalanceAfter = mockToken.balanceOf(address(briVault));
assertEq(vaultBalanceAfter, vaultBalanceBefore);
// Users still have their shares but can't use them
assertEq(briVault.balanceOf(user1), user1Shares);
assertEq(briVault.balanceOf(user2), user2Shares);
}

Recommended Mitigation

If no one bet on the winner, allow everyone to withdraw their stake.

function withdraw() external winnerSet {
if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
​
+ // If no one bet on the winner, allow everyone to withdraw their stake
+ if (totalWinnerShares == 0) {
+ uint256 refundAmount = stakedAsset[msg.sender];
+ if (refundAmount == 0) {
+ revert noDeposit();
+ }
+
+ uint256 shares = balanceOf(msg.sender);
+ stakedAsset[msg.sender] = 0;
+ _burn(msg.sender, shares);
+
+ IERC20(asset()).safeTransfer(msg.sender, refundAmount);
+ emit Withdraw(msg.sender, refundAmount);
+ return;
+ }
​
if (
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
keccak256(abi.encodePacked(winner))
) {
revert didNotWin();
}
uint256 shares = balanceOf(msg.sender);
​
uint256 vaultAsset = finalizedVaultAsset;
uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
_burn(msg.sender, shares);
​
IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
​
emit Withdraw(msg.sender, assetToWithdraw);
}
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!