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

Unbounded usersAddress Array Causes Gas DoS in setWinner() / _getWinnerShares()

minionteechs

Root + Impact

Description

  • Normal behavior:
    When the tournament ends, the owner should call setWinner() to finalize results and allow winning users to withdraw. The function internally calls _getWinnerShares() to sum all winning user shares efficiently.

  • Issue:

    The contract stores every participant’s address in usersAddress each time joinEvent() is called.
    Then, _getWinnerShares() loops over all entries in this array:
    Since usersAddress is unbounded, anyone can repeatedly call joinEvent() (or many addresses can join) to inflate the array indefinitely.
    When the owner later calls setWinner(), the transaction may run out of gas, permanently blocking finalization and all withdrawals — a Denial of Service.

// Root cause in the codebase with @> marks to highlight the relevant section
@> for (uint256 i = 0; i < usersAddress.length; ++i) {
@> address user = usersAddress[i];
@> totalWinnerShares += userSharesToCountry[user][winnerCountryId];
@> }
​

Risk

Likelihood:

  • Reason 1: This occurs whenever many users or a single attacker join the event multiple times, creating a large usersAddress array.

  • Reason 2: It also occurs naturally with high participation; gas grows linearly with array length, making setWinner() revert once array size exceeds gas limits.

Impact:

  • Prevents the owner from finalizing the tournament — locking all deposits and making withdrawals impossible.

  • Creates a permanent Denial of Service that halts the entire contract lifecycle.

Proof of Concept

An attacker can deliberately fill the usersAddress array to make setWinner() uncallable:

  1. Deploy the vault and start registration.

  2. Before eventStartDate, attacker repeatedly calls:

  3. The loop in _getWinnerShares() now iterates over 10 000+ entries.

  4. When the owner calls setWinner(), gas exceeds block limit → revert → contract frozen.

for (uint256 i = 0; i < 10_000; i++) {
vault.joinEvent(0); // same address joins many times
}
​

Recommended Mitigation

Use constant-time share aggregation instead of iterating through every participant.
Maintain a per-country total shares counter during joinEvent() and cancelParticipation().
Explanation:

Instead of calculating the winner’s total shares at the end with a for-loop, update a running counter whenever users join or leave.
This makes _getWinnerShares() O(1) instead of O(n).


- remove this code
+ add this code
+ mapping(uint256 => uint256) public totalSharesPerCountry;
+ mapping(address => bool) public hasJoined;
​
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) revert noDeposit();
if (countryId >= teams.length) revert invalidCountry();
if (block.timestamp > eventStartDate) revert eventStarted();
​
+ if (hasJoined[msg.sender]) revert notRegistered(); // prevent duplicates
+ hasJoined[msg.sender] = true;
​
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
+ totalSharesPerCountry[countryId] += participantShares; // update aggregate
usersAddress.push(msg.sender);
numberOfParticipants++;
totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}
​
- 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) {
+ totalWinnerShares = totalSharesPerCountry[winnerCountryId];
+ return totalWinnerShares;
+ }
​
Updates

Appeal created

bube Lead Judge 19 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!