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

DoS on setWinner() via unbounded iteration causes permanent vault lock

jopantech

[H-03] DoS on setWinner() via unbounded iteration causes permanent vault lock

Description

The function setWinner() calls _getWinnerShares(), which iterates over every address in the usersAddress array to sum their shares:

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

If the number of users (usersAddress.length) grows significantly (e.g., thousands of participants or Sybil addresses), this loop will exceed the block gas limit, causing setWinner() to revert.

As a result, the owner will never be able to finalize the tournament, and all funds within the vault will be permanently locked — creating a Denial of Service (DoS) scenario.

Risk

Likelihood: Medium

  • Although not every vault will reach thousands of participants, a Sybil attacker can deliberately create many small deposits to trigger this.

  • The issue stems from unbounded iteration in an on-chain loop.

Impact: High

  • setWinner() fails permanently once the array grows too large.

  • This blocks the entire finalization flow — rewards cannot be distributed, and user funds remain locked indefinitely.

  • The impact is system-wide and irreversible without redeployment or admin intervention.

Proof of Concept

The following Foundry test demonstrates how a large usersAddress array causes setWinner() to revert due to gas exhaustion:

function test_DoS_SetWinnerOutOfGas() public {
// Simulate many users joining
for (uint256 i = 0; i < 2000; ++i) {
address user = address(uint160(i + 1));
vm.deal(user, 1 ether);
vm.startPrank(user);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user);
vm.stopPrank();
}
​
// Attempt to set winner — should run out of gas
vm.startPrank(briVault.owner());
vm.expectRevert(); // out-of-gas due to large iteration
briVault.setWinner(1);
vm.stopPrank();
}

Run with:

forge test --match-test test_NoWinnerPickedDivisionByZero -vvv

Expected result:

[FAIL: EvmError: Revert] test_DoS_SetWinnerOutOfGas() (gas: 1073720587)

Recommended Mitigation

  • Avoid iterating over all participants during finalization. Instead, maintain an aggregated counter of total shares per country as deposits occur.

// Add this mapping
mapping(uint256 => uint256) public countryTotalShares;
​
// On joinEvent()
function joinEvent(uint256 countryId) external {
uint256 participantShares = balanceOf(msg.sender);
require(userSharesToCountry[msg.sender][countryId] == 0, "already joined");
userSharesToCountry[msg.sender][countryId] = participantShares;
+ countryTotalShares[countryId] += participantShares;
}
​
// On cancelParticipation()
function cancelParticipation(uint256 countryId) external {
uint256 sharesBurnt = userSharesToCountry[msg.sender][countryId];
userSharesToCountry[msg.sender][countryId] = 0;
+ countryTotalShares[countryId] -= sharesBurnt;
}
​
// _getWinnerShares() now becomes O(1)
function _getWinnerShares() internal view returns (uint256) {
- for (uint256 i = 0; i < usersAddress.length; ++i) {
- totalWinnerShares += userSharesToCountry[usersAddress[i]][winnerCountryId];
- }
- return totalWinnerShares;
+ return countryTotalShares[winnerCountryId];
}
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!