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

`BriVault::joinEvent` allows duplicate joins inflating participant counters

sulfurpt

Root + Impact

Description

  • The `joinEvent(uint256 countryId)` function allows the same address to join multiple times.

    Each call pushes `msg.sender` into `usersAddress` array, increments `numberOfParticipants` and increments `totalParticipantShares` by `balanceOf(msg.sender)`. However, `userSharesToCountry[msg.sender][countryId]` is overwritten rather than accumulated, leading to inflated totals and duplicate entries.

// BriVault::joinEvent
userSharesToCountry[msg.sender][countryId] = participantShares;
usersAddress.push(msg.sender);
numberOfParticipants++;
totalParticipantShares += participantShares;

Risk

Likelihood:

  • When users interact directly with the contract (via scripts, CLI, or malicious UI) and call joinEvent repeatedly from the same address, the counters increment on each call and totals diverge from per‑user records.

  • When a malicious actor automates multiple joinEvent calls across many controlled accounts, the usersAddress array increases rapidly and later owner operations that iterate or sum over that array face higher gas consumption or may fail due to gas limits.

Impact:

  • `numberOfParticipants` and `totalParticipantShares` can be inflated by repeated calls, breaking reward distribution logic.

  • Array growth can lead to high gas costs and potential DoS attacks.

  • Aggregate metrics no longer match individual share assignments.

Proof of Concept

1. User3 approves and deposits 2 ether.
2. User3 calls `joinEvent(1)` twice.
3. Observe that `numberOfParticipants` and `totalParticipantShares` increase incorrectly.

Add the following to `briVault.t.sol`

function test_POC_DuplicateJoin() public {
uint256 depositAmount = 2 ether;
​
// user3 approves and deposits
vm.prank(user3);
mockToken.approve(address(briVault), depositAmount);
​
vm.prank(user3);
briVault.deposit(depositAmount, user3);
​
// First join
vm.prank(user3);
briVault.joinEvent(1);
​
uint256 participantsAfterFirst = briVault.numberOfParticipants();
uint256 totalParticipantSharesAfterFirst = briVault.totalParticipantShares();
​
// Second join (duplicate)
vm.prank(user3);
briVault.joinEvent(1);
​
uint256 participantsAfterSecond = briVault.numberOfParticipants();
uint256 totalParticipantSharesAfterSecond = briVault.totalParticipantShares();
​
// Assertions
assertEq(participantsAfterSecond, participantsAfterFirst + 1, "numberOfParticipants should increase again (duplicate join)");
assertTrue(totalParticipantSharesAfterSecond > totalParticipantSharesAfterFirst, "totalParticipantShares increased even though userSharesToCountry overwritten");
​
// Confirm per-user shares remain unchanged
uint256 recordedShares = briVault.userSharesToCountry(user3, 1);
uint256 balanceShares = briVault.balanceOf(user3);
assertEq(recordedShares, balanceShares, "userSharesToCountry equals balanceOf (but totalParticipantShares inflated)");
}

Recommended Mitigation

Add a duplicate-join guard to prevent multiple joins per address:

+ error AlreadyJoined();
​
mapping(address => bool) public hasJoined;
​
function joinEvent(uint256 countryId) public {
if (hasJoined[msg.sender]) revert AlreadyJoined();
+ hasJoined[msg.sender] = true;
// Existing logic
}

Optionally, maintain per-country totals instead of iterating over `usersAddress` to reduce gas costs:

mapping(uint256 => uint256) public countryTotalShares;
​
function joinEvent(uint256 countryId) public {
...
userSharesToCountry[msg.sender][countryId] = participantShares;
countryTotalShares[countryId] += participantShares;
...
}
​
function _getWinnerShares() internal view returns (uint256) {
- uint256 totalWinnerShares = 0;
- for (uint256 i = 0; i < usersAddress.length; ++i) {
- totalWinnerShares += userSharesToCountry[usersAddress[i]][winnerCountryId];
- }
- return totalWinnerShares;
+ return countryTotalShares[winnerCountryId];
}
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!