BriVault

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

Potential Gas Denial-of-Service in _getWinnerShares

Root + Impact

Description

  • The _getWinnerShares function aims to sum shares for the winning country by iterating over all unique participant addresses once.

With potential duplicates (from other issues) or high participation, the unbounded loop over usersAddress consumes excessive gas, risking block limit exceedance.

function _getWinnerShares () internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i){ // @> O(n) over potentially large/duplicated array
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
// ...
}

Risk

Likelihood:

  • Post-event with 10,000+ participants filling the array.

Amplified by duplicate pushes from repeated joins, pushing n > 30k entries.

Impact:

  • setWinner reverts on gas limit, indefinitely blocking winner announcements and withdrawals.

DoS enables griefers to spam joins pre-start, halting the entire event payout.

Proof of Concept

// Simulate 10k users joining (or fewer with duplicates)
function testGasDoS() public {
// Loop to add 10000 entries (e.g., 1000 users x 10 duplicates each)
for (uint i = 0; i < 10000; i++) {
vm.prank(user); vault.joinEvent(0); // Duplicates bloat
}
// setWinner: loop iterates 10k times, each += operation; gas > 30M, reverts
vm.expectRevert(); // Gas exhaustion
vm.prank(owner); vault.setWinner(0);
}

Recommended Mitigation

// Replace array loop with per-country total mapping
+mapping(uint256 => uint256) public countryTotalShares;
function joinEvent(uint256 countryId) public {
// ...
- usersAddress.push(msg.sender); // Remove array
+ countryTotalShares[countryId] += participantShares; // Aggregate here
// ...
}
function _getWinnerShares () internal returns (uint256) {
- for (uint256 i = 0; i < usersAddress.length; ++i){
- address user = usersAddress[i];
- totalWinnerShares += userSharesToCountry[user][winnerCountryId];
- }
- return totalWinnerShares;
+ return totalWinnerShares = 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!