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

`BriVault::setWinner` can run out of gas for large number of participants (Gas DoS)

sulfurpt

Root + Impact

Description

  • `The setWinner()` function iterates over all participants to calculate total winner shares

  • When the number of participants is very large, this loop consumes a high amount of gas. This can cause the transaction to run out of gas, effectively preventing the owner from setting the winner and finalizing the vault.

// BriVault::setWinner
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId];

Risk

Impact:

  • For a large number of participants, `setWinner()` may revert due to gas limits, creating a Denial-of-Service (DoS) scenario.

  • No direct theft of funds occurs, but participants may be blocked from withdrawing their rewards until the issue is resolved.

  • Risk is higher in events with thousands of participants, while small events are unaffected.

Proof of Concept

1. Create 5,000 fake participants and have each deposit + join the event.
2. Warp time to after the event ends.
3. Owner calls setWinner().
4. Transaction reverts due to gas limit.

Add the following to `briVault.t.sol`

function test_POC_SetWinnerGasDoS() public {
// create N fake users and have them deposit + join (N large)
for (uint i=0; i<5000; i++) {
address a = makeAddr(string(abi.encodePacked("u", vm.toString(i))));
mockToken.mint(a, 1 ether);
vm.prank(a);
mockToken.approve(address(briVault), 1 ether);
vm.prank(a);
briVault.deposit(1 ether, a);
vm.prank(a);
briVault.joinEvent(1);
}
​
// gas consumed by setWinner
uint256 gasStart = gasleft();
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(1);
uint256 gasUsed = gasStart - gasleft();
​
console.log("Gas consumed by setWinner with 5000 users:", gasUsed);
​
​
}

Recommended Mitigation

Avoid iterating over unbounded arrays on-chain and consider batch processing, mapping-based aggregation, or off-chain computation to calculate winner shares.

- for (uint256 i = 0; i < usersAddress.length; ++i){
- address user = usersAddress[i];
- totalWinnerShares += userSharesToCountry[user][winnerCountryId];
- }
+ // Possible mitigation:
+ // 1. Maintain a running total per country on joinEvent()
+ // 2. Or split setWinner() into multiple transactions to handle batches
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!