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

Unbounded Participant Array Allows Denial-of-Service and Reward Manipulation via Repeated joinEvent() Calls

pushprakash23

Root + Impact

Description

  • Normal behavior:
    Users should be able to join the event only once after depositing. The contract should store each participant uniquely and later aggregate shares to calculate rewards for the winning country.

  • Issue:
    The joinEvent() function pushes msg.sender into the usersAddress array without checking for duplicates. Attackers can repeatedly call joinEvent() before the event starts, inflating the array. During setWinner(), the contract loops through this array in _getWinnerShares(), causing potential gas exhaustion (DoS) or incorrect share calculations due to duplicate counting.

// Root cause
function joinEvent(uint256 countryId) public {
...
// @> No uniqueness check — attacker can spam this
usersAddress.push(msg.sender);
}
​
function _getWinnerShares () internal returns (uint256) {
// @> Iterates over all user entries, duplicates counted multiple times
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
}
​

Risk

Likelihood:

  • A participant can repeatedly call joinEvent() after a single deposit, since no guard limits re-entry.

  • The array usersAddress grows without restriction, and iteration in _getWinnerShares() depends directly on user-controlled input.

Impact:

  • The setWinner() function can run out of gas and fail, permanently locking vault finalization and withdrawals.


Duplicate entries inflate totalWinnerShares, breaking payout fairness and allowing reward manipulation.

Proof of Concept

  1. Sets up a local Foundry test instance, deploying a mock token and the BriVault.

  2. Mints tokens to an attacker address and has the attacker deposit once.

  3. The attacker spam-calls joinEvent(0) 3,000 times (via vm.prank(attacker) so each call appears from the attacker).

  4. The test advances time past the eventEndDate and expects vault.setWinner(0) to revert.

// Example PoC (pseudo test)
vault.deposit(minAmount, attacker);
for (uint256 i = 0; i < 5000; i++) {
vault.joinEvent(0); // attacker spams joins
}
// When owner calls:
vault.setWinner(0); // -> reverts due to gas exhaustion in _getWinnerShares()
​

Recommended Mitigation

  1. Enforce single join per user.

  2. Replace user iteration with per-country aggregation

  3. Fix deposit mint mismatch

- remove this code
+ add this code
​
+mapping(address => bool) public joined;
​
+function joinEvent(uint256 countryId) public {
require(!joined[msg.sender], "Already joined");
joined[msg.sender] = true;
...
}
​
​
+mapping(uint256 => uint256) public countryTotalShares;
// Update on join/cancel instead of looping users
​
​
+_mint(receiver, participantShares);
​
function _getWinnerShares () internal returns (uint256) {
- for (uint256 i = 0; i < usersAddress.length; ++i){
- address user = usersAddress[i];
- totalWinnerShares += userSharesToCountry[user][winnerCountryId];
- }
- return totalWinnerShares;
+ totalWinnerShares = countryTotalShares[winnerCountryId];
+ return totalWinnerShares;
}
​
​
Updates

Appeal created

bube Lead Judge 21 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.

pushprakash23 Submitter
21 days ago
bube Lead Judge
18 days ago
bube Lead Judge 17 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!