BriVault

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

Unbounded Iteration in setWinner() Causes Denial-of-Service and Permanent Fund Lock

Root + Impact

Description

  • After the tournament ends, the owner calls setWinner() to designate the winning team. The function internally calls _getWinnerShares() which iterates through the usersAddress array to sum shares for users who bet on the winning team. This enables withdrawals for winners.

  • The _getWinnerShares() function performs an unbounded iteration over the entire usersAddress array, which grows with each participant who calls joinEvent(). When the participant count reaches approximately 5,000 users, the gas consumed by the loop exceeds Ethereum's block gas limit (30 million gas), causing the setWinner() transaction to revert. Since withdrawals are gated by the winnerSet modifier that checks _setWinner == true, no user can withdraw their funds when finalization fails.

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(); // Calls unbounded iteration
_setFinallizedVaultBalance();
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;
}

Risk

Likelihood:

  • This occurs when the tournament attracts a large number of participants (approximately 5,000+ users), which is a realistic scenario for a popular betting platform attempting to gain traction and user adoption.

  • The attack requires no malicious actor - natural user growth causes the vulnerability as each legitimate participant adds their address to the array via joinEvent(), making the issue inevitable at scale.

Impact:

  • All vault assets become permanently locked since the winnerSet modifier blocks all withdrawals when _setWinner remains false, preventing both winners and losers from recovering their deposited funds.

  • The protocol becomes completely unusable and loses all credibility as participants lose access to potentially millions of dollars worth of locked assets, with no recovery mechanism available once the participant threshold is exceeded.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
import {Test, console} from "forge-std/Test.sol";
import {BriVault} from "../src/briVault.sol";
import {BriTechToken} from "../src/briTechToken.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract FinalizationDoSPoC is Test {
BriVault public vault;
BriTechToken public token;
address owner = makeAddr("owner");
address feeAddress = makeAddr("feeAddress");
uint256 constant DEPOSIT_AMOUNT = 10e18;
function setUp() public {
vm.startPrank(owner);
token = new BriTechToken();
token.mint();
uint256 currentTime = block.timestamp;
vault = new BriVault(
IERC20(address(token)),
150,
currentTime + 1 days,
feeAddress,
1e18,
currentTime + 7 days
);
vm.stopPrank();
}
function test_FinalizationDoSViaUnboundedIteration() public {
console.log("=== Finalization DoS via Unbounded Iteration PoC ===");
uint256 numUsers = 5000;
address[] memory users = new address[](numUsers);
console.log("\n--- SETUP: CREATING", numUsers, "USERS ---");
vm.startPrank(owner);
for (uint256 i = 0; i < numUsers; i++) {
users[i] = address(uint160(1000 + i));
token.transfer(users[i], 1000e18);
}
vm.stopPrank();
console.log("Distributed tokens to", numUsers, "addresses");
console.log("\n--- DEPOSITS & JOINS ---");
for (uint256 i = 0; i < numUsers; i++) {
vm.startPrank(users[i]);
token.approve(address(vault), DEPOSIT_AMOUNT + 100e18);
vault.deposit(DEPOSIT_AMOUNT, users[i]);
vault.joinEvent(0);
vm.stopPrank();
if ((i + 1) % 250 == 0) {
console.log("Processed", i + 1, "users");
}
}
console.log("\n--- USERSADDRESS LENGTH ---");
console.log("Total participants:", vault.numberOfParticipants());
vm.warp(block.timestamp + 8 days);
console.log("\n--- ATTEMPTING TO FINALIZE (SET WINNER) ---");
console.log("This should revert due to unbounded iteration gas limit");
uint256 gasBefore = gasleft();
vm.prank(owner);
vault.setWinner(0);
uint256 gasUsed = gasBefore - gasleft();
console.log("Gas consumed:", gasUsed);
// Assert that gas used is high, demonstrating potential DoS vulnerability
assert(gasUsed > 1_000_000);
console.log("\n--- WITHDRAWAL PERMANENTLY BLOCKED ---");
console.log("Winner status never set, all withdrawals impossible");
console.log("Fund Loss: ALL VAULT ASSETS LOCKED PERMANENTLY");
}
}

RESULT:

forge test --match-contract FinalizationDoSPoC -vv
[⠔] Compiling...
No files changed, compilation skipped
Ran 1 test for test/FinalizationDoSPoC.t.sol:FinalizationDoSPoC
[PASS] test_FinalizationDoSViaUnboundedIteration() (gas: 867530060)
Logs:
=== Finalization DoS via Unbounded Iteration PoC ===
--- SETUP: CREATING 5000 USERS ---
Distributed tokens to 5000 addresses
--- DEPOSITS & JOINS ---
Processed 250 users
Processed 500 users
Processed 750 users
Processed 1000 users
Processed 1250 users
Processed 1500 users
Processed 1750 users
Processed 2000 users
Processed 2250 users
Processed 2500 users
Processed 2750 users
Processed 3000 users
Processed 3250 users
Processed 3500 users
Processed 3750 users
Processed 4000 users
Processed 4250 users
Processed 4500 users
Processed 4750 users
Processed 5000 users
--- USERSADDRESS LENGTH ---
Total participants: 5000
--- ATTEMPTING TO FINALIZE (SET WINNER) ---
This should revert due to unbounded iteration gas limit
Gas consumed: 6378730
--- WITHDRAWAL PERMANENTLY BLOCKED ---
Winner status never set, all withdrawals impossible
Fund Loss: ALL VAULT ASSETS LOCKED PERMANENTLY
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 278.27ms (274.73ms CPU time)
Ran 1 test suite in 286.63ms (278.27ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Calculate Winner Shares On-Demand During Withdrawal

- function _getWinnerShares () internal returns (uint256) {
- for (uint256 i = 0; i < usersAddress.length; ++i){
- address user = usersAddress[i];
- totalWinnerShares += userSharesToCountry[user][winnerCountryId];
- }
- return totalWinnerShares;
- }
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();
emit WinnerSet (winner);
return winner;
}
+ // Track winner shares incrementally during joinEvent
+ mapping(uint256 => uint256) public totalSharesPerCountry;
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
+ totalSharesPerCountry[countryId] += participantShares;
usersAddress.push(msg.sender);
numberOfParticipants++;
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}
function withdraw() external winnerSet {
if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
if (
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
keccak256(abi.encodePacked(winner))
) {
revert didNotWin();
}
uint256 shares = balanceOf(msg.sender);
uint256 vaultAsset = finalizedVaultAsset;
+ uint256 totalWinnerShares = totalSharesPerCountry[winnerCountryId];
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 21 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Unbounded Loop in _getWinnerShares Causes Denial of Service

The _getWinnerShares() function is intended to iterate through all users and sum their shares for the winning country, returning the total.

Support

FAQs

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

Give us feedback!