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

Unbounded Loop in _getWinnerShares Causes Denial of Service

comfortnurse021

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

Problem:
The function loops through the entire usersAddress array without bounds or batching, and writes to a state variable (totalWinnerShares), compounding gas costs.
Once the user array grows large, this function can no longer execute successfully due to block gas limit constraints.

https://github.com/CodeHawks-Contests/2025-11-brivault/blob/1f515387d58149bf494dc4041b6214c2546b3b27/src/briVault.sol#L191-198

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

Risk

Risk

Likelihood:

  • The loop executes every time _getWinnerShares() is called, regardless of array size.

  • As usersAddress grows, gas consumption increases linearly until it exceeds the block gas limit.

Impact:

  • Functions depending on _getWinnerShares() will revert, halting reward distribution or result settlement.

  • Contract becomes non-functional for large user bases (DoS against entire protocol).

Proof of Concept

pragma solidity ^0.8.20;
import "forge-std/Test.sol";
​
contract WinnerShareTest is Test {
address[] public usersAddress;
mapping(address => mapping(uint256 => uint256)) public userSharesToCountry;
uint256 public totalWinnerShares;
uint256 public winnerCountryId = 1;
​
function setUp() public {
// Simulate thousands of users
for (uint256 i = 0; i < 20000; i++) {
address user = address(uint160(i + 1));
usersAddress.push(user);
userSharesToCountry[user][winnerCountryId] = 1;
}
}
​
function _getWinnerShares() internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}
​
function testExploit() public {
// Expect this call to run out of gas (simulate DoS)
vm.expectRevert();
_getWinnerShares();
}
}
Why Pick This PoC
​
Because it realistically reproduces the failure condition:
​
Creates a large user array (20,000 entries).
​
Calls the vulnerable function.
​
Demonstrates that the transaction cannot complete — validating the DoS impact.

Recommendation

  • function getWinnerSharesBatch(uint256 start, uint256 end) external view returns (uint256 batchTotal) {
    require(end <= usersAddress.length, "Invalid range");
    for (uint256 i = start; i < end; ++i) {
    address user = usersAddress[i];
    batchTotal += userSharesToCountry[user][winnerCountryId];
    }
    }

  • What This Does

    Instead of processing every user in one transaction, it processes only a small range (start → end).

    You can call it multiple times (or off-chain) to sum the results in parts.

Updates

Appeal created

bryanconquer 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!