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

Lack of duplicate-user validation in `joinEvent()` allows attackers to inflate participant counts, distort `totalParticipantShares`, and manipulate winner payout distribution.

xcryptoguardian

Description:

The joinEvent() function blindly pushes msg.sender into usersAddress every time it is called:

// @audit duplicate user added repeatedly
usersAddress.push(msg.sender);

There is no check preventing a single address from calling joinEvent() multiple times:

  1. usersAddress grows with repeated duplicates

  2. totalParticipantShares increases multiple times for the same participant

  3. numberOfParticipants increments misleadingly

  4. totalWinnerShares becomes inflated because _getWinnerShares() iterates through every occurrence of the duplicated address

  5. withdraw() then pays out multiple times more than intended

Because _getWinnerShares() loops over the array and sums:

totalWinnerShares += userSharesToCountry[user][winnerCountryId];

Thus, a malicious user can drastically reduce totalWinnerShares (by being the only winner) OR inflate their own influence—both resulting in stolen funds or skewed distribution.
This breaks the core vault math and lets an attacker receive more tokens than they deposited.

Likelihood:

  • Reason 1: joinEvent() is a public function with no protections; any user can call it multiple times without restrictions.

  • Reason 2: No state variable checks if a user has already joined, and no mapping tracks whether a user has previously selected a country.

Impact:

  • Impact 1: Attackers inflate totalParticipantShares, numberOfParticipants, and usersAddress artificially, breaking winner-share math.

  • Impact 2: _getWinnerShares() sums the same user's shares multiple times, allowing attackers to manipulate payouts and extract a disproportionate share of the vault’s rewards.

Severity: HIGH (incorrect accounting → loss of user funds / protocol manipulation)

Proof of Concept:

Add this PoC to the existing test suite.
It shows one user joining 3 times and tripling their effective winner-shares, giving them a massively inflated payout.

PoC
function test_DuplicateUserManipulation() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
// User deposits once
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
uint256 shares = briVault.deposit(1 ether, user1);
briVault.joinEvent(0); // First join
vm.stopPrank();
​
// Duplicate joins
vm.startPrank(user1);
briVault.joinEvent(0); // Second join (incorrectly allowed)
briVault.joinEvent(0); // Third join
vm.stopPrank();
​
// Warp to end and finalize winner
vm.warp(eventEndDate + 1);
​
vm.startPrank(owner);
briVault.setWinner(0);
vm.stopPrank();
​
// Withdraw as winner
uint256 before = mockToken.balanceOf(user1);
​
vm.startPrank(user1);
briVault.withdraw();
vm.stopPrank();
​
uint256 after = mockToken.balanceOf(user1);
​
console.log("User shares:", shares);
console.log("Effective winner shares counted:", briVault.totalWinnerShares());
console.log("User received:", after - before);
​
// The user should receive exactly `1 ether` (minus fee),
// but due to duplicate join entries, they receive much more.
assertGt(after - before, shares, "Duplicate join inflated payout");
}
​

What the test proves

User joins 3 times

  • _getWinnerShares() counts the same user's shares 3 times

  • StotalWinnerShares is 3× higher

  • Reward formula becomes manipulated

  • User receives excess tokens

  • Duplicate entries directly cause an overpayment.

Recommended Mitigation:

Option 1 — Prevent duplicate joins (preferred)
Add a boolean mapping:

mapping(uint256 => uint256) totalSharesByCountry;

And modify joinEvent():

if (hasJoined[msg.sender]) revert AlreadyJoined();
​
hasJoined[msg.sender] = true;
usersAddress.push(msg.sender);

Option 2 — Replace array with correct accounting
Remove usersAddress entirely and track:

mapping(uint256 => uint256) totalSharesByCountry;

Update on join:

totalSharesByCountry[countryId] += participantShares;

Then compute winner shares instantly:

totalWinnerShares = totalSharesByCountry[winnerCountryId];

Option 3 — Allow re-joining but update instead of pushing
If multiple join-attempts should overwrite:

if (!hasJoined[msg.sender]) {
usersAddress.push(msg.sender);
hasJoined[msg.sender] = true;
}
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!