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

Duplicate Participation in joinEvent() Causes Double Counting of Shares and Participants

sidd

Root + Impact

Description

  • Normal behavior: A user deposits assets, then calls joinEvent(countryId) once to register themselves for a single team. The contract should record the user's chosen team, their shares for that team, and update global participant counters only once per unique user.

  • Issue: joinEvent() currently allows the same address to call it multiple times before the event starts. Each call overwrites userToCountry, writes userSharesToCountry[msg.sender][countryId], pushes the address into usersAddress unconditionally, and increments numberOfParticipants and totalParticipantShares without checking prior participation. This results in duplicate entries and double-counting of shares and participants.

function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
// ... some validations
​
userToCountry[msg.sender] = teams[countryId];
​
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
@> usersAddress.push(msg.sender); // duplicate pushes allowed
@> numberOfParticipants++; // double-counting participants
@> totalParticipantShares += participantShares; // double-counting shares
​
emit joinedEvent(msg.sender, countryId);
}

Risk

Likelihood:

  • Users or scripts that call joinEvent() more than once (due to UX retries, bot mistakes, or malicious action) will trigger duplicate entries because the function lacks a hasJoined guard.

  • Off-chain tools or owners may allow team switching workflows that call joinEvent() multiple times without removing previous entries, leading to duplicated state.

  • Tests and _getWinnerShares() iterate usersAddress and sum per-address values — duplicates in usersAddress will be counted multiple times during winner share calculation.

Impact:

  • Redistribution math becomes wrong: totalWinnerShares and totalParticipantShares can be inflated, causing underpayment or overpayment to winners.

  • numberOfParticipants becomes inaccurate, misleading analytics and off-chain reporting.

  • Gas cost increase and potential out-of-gas loops when iterating a bloated usersAddress.

  • In extreme cases, an attacker could intentionally call joinEvent() many times (from many accounts or via repeated calls) to distort payouts or Deny/Disrupt correct settlement.

Proof of Concept

The following Foundry test demonstrates how calling joinEvent() twice results in duplicate participant entries and inflated share totals.
The same address is added to usersAddress multiple times, increasing both numberOfParticipants and totalParticipantShares without adding new deposits.

function test_double_join() public {
// 1. Owner sets countries
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
// 2. user1 deposits
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
vm.stopPrank();
​
// 3. user1 joins the same country twice
vm.startPrank(user1);
briVault.joinEvent(10); // first join
briVault.joinEvent(10); // duplicate join
vm.stopPrank();
​
// 4. Advance time and set winner to trigger aggregation
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(10);
vm.stopPrank();
​
// 5. Log key state variables
console.log("numberOfParticipants:", briVault.numberOfParticipants());
console.log("totalParticipantShares:", briVault.totalParticipantShares());
console.log("userSharesToCountry[user1][10]:", briVault.userSharesToCountry(user1, 10));
console.log("totalWinnerShares:", briVault.totalWinnerShares());
console.log("=== DOUBLE-JOIN-DEBUG-END ===");
}

Observed Results :

=== AFTER FIRST JOIN ===
numberOfParticipants: 1
totalParticipantShares: 4925000000000000000
userSharesToCountry[user1][10]: 4925000000000000000
userToCountry[user1]: Japan
​
=== AFTER SECOND JOIN (DUPLICATE) ===
numberOfParticipants: 2
totalParticipantShares: 9850000000000000000
userSharesToCountry[user1][10]: 4925000000000000000
userToCountry[user1]: Japan
​
=== AFTER setWinner(10) ===
finalizedVaultAsset: 4925000000000000000
totalWinnerShares: 9850000000000000000

Explanation:
The logs clearly show that after a duplicate joinEvent() call:

  • numberOfParticipants increased from 1 → 2

  • totalParticipantShares doubled from 4.925 ETH → 9.85 ETH,
    even though user1 joined with the same shares and deposit.

When setWinner(10) runs, _getWinnerShares() aggregates from the duplicated usersAddress entries, double-counting the same user’s shares and inflating total rewards distribution.
This confirms a state integrity and accounting vulnerability in the current implementation.

Recommended Mitigation

@@
error WinnerAlreadySet();
+ error alreadyJoined(); // new: revert when user tries to join more than once
error limiteExceede();
@@
mapping(address => uint256) public stakedAsset;
mapping(address => string) public userToCountry;
mapping(address => mapping(uint256 => uint256)) public userSharesToCountry;
+ mapping(address => bool) public hasJoined; // new: track if address already joined
@@
- function joinEvent(uint256 countryId) public {
+ function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
// Ensure countryId is a valid index in the `teams` array
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
-
- userToCountry[msg.sender] = teams[countryId];
-
- uint256 participantShares = balanceOf(msg.sender);
- userSharesToCountry[msg.sender][countryId] = participantShares;
-
- usersAddress.push(msg.sender);
-
- numberOfParticipants++;
- totalParticipantShares += participantShares;
-
- emit joinedEvent(msg.sender, countryId);
+ // Prevent duplicate joins — require user not already registered
+ if (hasJoined[msg.sender]) {
+ revert alreadyJoined();
+ }
+
+ userToCountry[msg.sender] = teams[countryId];
+ uint256 participantShares = balanceOf(msg.sender);
+ userSharesToCountry[msg.sender][countryId] = participantShares;
+
+ // record participant once
+ usersAddress.push(msg.sender);
+ hasJoined[msg.sender] = true;
+
+ numberOfParticipants++;
+ totalParticipantShares += participantShares;
+
+ emit joinedEvent(msg.sender, countryId);
}
​

The fix introduces a simple hasJoined mapping and an alreadyJoined() error to stop users from joining the same event multiple times. Before recording a new participant, joinEvent() now checks if the user has already joined and reverts if true. This ensures that each address is counted only once, preventing inflated totals in numberOfParticipants, totalParticipantShares, and totalWinnerShares. The change keeps the logic clean, cheap, and accurate without affecting normal user flow.

Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Duplicate registration through `joinEvent`

Support

FAQs

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

Give us feedback!