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

Unbounded iteration in _getWinnerShares() can cause out-of-gas (OOG) errors

eze001

Root + Impact

Description

  • Normally, _getWinnerShares() should efficiently determine the winner’s reward based on stored participant data without risking execution failure.

  • However, the current implementation loops over all participants in the array, which grows indefinitely as more users join. This creates an unbounded gas cost, making the function potentially unusable once the participant list becomes large.

// Root cause in the codebase with @> marks to highlight the relevant section
function _getWinnerShares() internal view returns (uint256 totalWinnerShares) {
@> for (uint256 i = 0; i < participants.length; i++) {
@> totalWinnerShares += participants[i].shares;
}
}
​

Risk

Likelihood:

  • Very likely to occur as soon as the number of participants grows beyond the gas limit per transaction.

  • The issue compounds with time as the list accumulates entries from repeated deposits or events.

Impact:

  • Any function calling _getWinnerShares() may become unexecutable, halting key operations like reward distribution or event closure.

  • The contract may effectively “brick” if _getWinnerShares() is required for payouts or accounting.

Proof of Concept

Observed Effect:
When joinMany(10000) is called, the calculate() function fails due to out-of-gas errors during execution.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
contract PoC_UnboundedLoop {
uint256[] public participants;
​
function joinMany(uint256 count) external {
for (uint256 i = 0; i < count; i++) {
participants.push(i);
}
}
​
function calculate() external view returns (uint256 total) {
for (uint256 i = 0; i < participants.length; i++) {
total += participants[i]; // gas cost grows linearly
}
}
}
​

Recommended Mitigation

**Explanation: **Instead of recalculating the total each time, maintain a running counter (totalDepositedShares) updated during deposits and withdrawals. This approach makes _getWinnerShares() an O(1) operation, eliminating the risk of gas exhaustion.

function _getWinnerShares() internal view returns (uint256 totalWinnerShares) {
- for (uint256 i = 0; i < participants.length; i++) {
- totalWinnerShares += participants[i].shares;
- }
+ // Store and update the total shares during each participant deposit
+ // Instead of looping, simply read the stored total value
+ totalWinnerShares = totalDepositedShares;
}
​
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!