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

usersAddress Not Cleaned / Duplicate Joins Allowed (inflates counts & gas)

chobby

Duplicate joinEvent() calls and never removal of usersAddress cause inflated counts, gas growth, and broken winner calculations.

Description

  • The joinEvent() function appends msg.sender to usersAddress every time it is called and does not check whether the user has already joined.

    Additionally, cancelParticipation() never removes or marks the user as inactive

  • This means a single address can appear multiple times in the participants list. When winner calculations or aggregate loops iterate through this array, shares are counted multiple times, breaking fairness and potentially making the vault unfinalizable due to gas exhaustion.

// BriVault.sol
​
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) { revert noDeposit(); }
// @> ROOT CAUSE: NO CHECK FOR DUPLICATE JOIN @>
​
// ... sets userToCountry and userSharesToCountry ...
​
usersAddress.push(msg.sender); // Appends the same user repeatedly
numberOfParticipants++;
// ...
}
​
function cancelParticipation() public {
// ... logic ...
// @> ROOT CAUSE: USER IS NEVER REMOVED OR MARKED INACTIVE @>
// usersAddress still contains the user, causing them to be counted in _getWinnerShares loop.
}
​

Risk

Likelihood:

  • Reason 1: The flaw is guaranteed to occur whenever a user mistakenly or maliciously calls joinEvent() more than once, as there is no guard to prevent it.


  • Reason 2: The corruption of state is irreversible once the setWinner() function has compounded the counts.

Impact:

  • Inflated numberOfParticipants and totalWinnerShares.


  • Incorrect winner payouts (skewed denominators reduce everyone’s payout).


  • Potential DoS if usersAddress grows too large.

Proof of Concept

This test proves that a user can join multiple times without depositing again, inflating participant counts and breaking payout logic.

function test_duplicate_join_inflates_users_array_and_winnerShares() public {
vm.prank(alice);
mockToken.approve(address(briVault), 10 ether);
vm.prank(alice);
briVault.deposit(10 ether, alice);
​
// First join
vm.prank(alice);
briVault.joinEvent(5);
​
// Second join - no revert, adds same user twice
vm.prank(alice);
briVault.joinEvent(5);
​
vm.warp(block.timestamp + 39 days);
vm.prank(owner);
briVault.setWinner(5);
​
vm.prank(alice);
briVault.withdraw();
​
// Alice appears twice in usersAddress
assertEq(briVault.numberOfParticipants(), 2);
}

Recommended Mitigation


Introduce a hasJoined mapping and revert duplicate joins.

Optionally mark users inactive when they cancel participation.


This finding overlaps conceptually with “Improper stakeAsset state handling”, since both stem from the lack of per-user lifecycle control.

However, while that issue focuses on incorrect stake overwriting and refund logic, this one focuses on behavioral abuse: allowing multiple joins with or without new deposits, which corrupts participant accounting.

+ mapping(address => bool) public hasJoined;
+ error AlreadyJoined();
​
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) { revert noDeposit(); }
+ if (hasJoined[msg.sender]) { revert AlreadyJoined(); }
​
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;
​
+ hasJoined[msg.sender] = true;
emit joinedEvent(msg.sender, countryId);
}
​
function cancelParticipation() public {
...
+ hasJoined[msg.sender] = false;
}
​
Updates

Appeal created

bube Lead Judge 20 days ago
Submission Judgement Published
Validated
Assigned finding tags:

`cancelParticipation` Leaves Stale Winner Data

CancelParticipation burns shares but leaves the address inside usersAddress and keeps userSharesToCountry populated.

Duplicate registration through `joinEvent`

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!