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

M02. Duplicate Joins and Double-Counting in Winner Share Calculation

accessdenied

Root + Impact

Description

  • Under normal operation, each participant is expected to join the event once, associating their deposited shares with a single country. At the end of the event, the contract aggregates total winning shares to distribute rewards fairly among participants who selected the winning country.

  • The issue is that joinEvent() allows users to call it multiple times, pushing their address repeatedly into usersAddress and incrementing counters each time. _getWinnerShares() then iterates over this array and adds each user’s shares to totalWinnerShares without resetting or checking for duplicates. This causes share double-counting, inflated totals, distorted payouts, and potential denial of service.

function joinEvent(uint256 countryId) public {
...
@> usersAddress.push(msg.sender);
@> numberOfParticipants++;
@> totalParticipantShares += participantShares;
...
}
​
function _getWinnerShares () internal returns (uint256) {
@> for (uint256 i = 0; i < usersAddress.length; ++i){
@> address user = usersAddress[i];
@> totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}
​

Risk

Likelihood:

  • Users can repeatedly call joinEvent() before the event starts, since there is no guard against multiple joins.

  • The owner must call setWinner() at the end of the event, which triggers _getWinnerShares(). This function loops through all entries in usersAddress, meaning any user can cause the total computation or gas cost to explode.

Impact:

  • Double-counting of shares inflates totalWinnerShares, causing incorrect withdrawal payouts (winners receive less per share).

  • Excessive entries in usersAddress can cause out-of-gas denial of service when calling setWinner(), preventing event finalization and locking user funds.

Proof of Concept

// Attacker deposits and calls joinEvent multiple times:
vault.deposit(100 ether, attacker);
vault.joinEvent(0); // First join
vault.joinEvent(0); // Second join
vault.joinEvent(0); // Third join
// Each call adds attacker address to usersAddress and increments participant count
​
// Later when owner calls setWinner(0):
// _getWinnerShares() loops over usersAddress
// Attacker’s shares are added three times into totalWinnerShares
​
// Result: totalWinnerShares is inflated, reducing everyone’s payout ratio
​

Explanation:
Since _getWinnerShares() sums shares for every entry in usersAddress, the attacker’s balance is added multiple times. Because withdrawal amount is computed as assetToWithdraw = shares * vaultAsset / totalWinnerShares, the inflated denominator reduces everyone’s payout, even though the attacker’s actual share count did not increase.

Recommended Mitigation

revent duplicate joins by enforcing a one-time participation per user or per country. Track join state in joinEvent() and update _getWinnerShares() to reset totalWinnerShares before aggregation.

Functions updated: joinEvent, _getWinnerShares

function joinEvent(uint256 countryId) public {
- usersAddress.push(msg.sender);
- numberOfParticipants++;
- totalParticipantShares += participantShares;
​
+ require(userSharesToCountry[msg.sender][countryId] == 0, "Already joined");
+ userSharesToCountry[msg.sender][countryId] = participantShares;
+ usersAddress.push(msg.sender);
+ numberOfParticipants++;
+ totalParticipantShares += participantShares;
​
}
function _getWinnerShares() internal returns (uint256) {
- for (uint256 i = 0; i < usersAddress.length; ++i){
- address user = usersAddress[i];
- totalWinnerShares += userSharesToCountry[user][winnerCountryId];
- }
+ totalWinnerShares = 0;
+ for (uint256 i = 0; i < usersAddress.length; ++i){
+ address user = usersAddress[i];
+ totalWinnerShares += userSharesToCountry[user][winnerCountryId];
+ }
}
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.

Duplicate registration through `joinEvent`

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!