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

Multiple User Addition to `usersAddress` Array

balakchiev

Root + Impact

Description

  • Describe the normal behavior in one or more sentences

Users should join an event once, with their address recorded uniquely in usersAddress. numberOfParticipants should reflect unique participants only.

  • Explain the specific issue or problem in one or more sentences

joinEvent() (line 263) lacks protection against multiple calls. Users can call it repeatedly, causing duplicate entries in usersAddress, inflated numberOfParticipants, and incorrect totalWinnerShares calculation as the same user's shares are counted multiple times.

// Root cause in the codebase with @> marks to highlight the relevant section
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
​
// Ensure countryId is a valid index in the `teams` array
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); // Missing check for existing participant
numberOfParticipants++;
totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

A user intentionally calls joinEvent() multiple times to manipulate participant counts or gain unfair advantage.

  • Reason 2

Accidental double-clicks, front-end bugs, or retry mechanisms cause multiple calls, corrupting state.

Impact:

  • Impact 1

Incorrect totalWinnerShares calculation causes unfair fund distribution. Same user's shares counted multiple times.

  • Impact 2

Increased gas costs from duplicate entries and unreliable numberOfParticipants counter affecting dependent functions.

Proof of Concept

briVault.deposit(5 ether, user);
briVault.joinEvent(10); // First call - adds user
briVault.joinEvent(20); // Second call - same user added again
// numberOfParticipants = 2, but only 1 unique user
// _getWinnerShares() counts same user twice

Recommended Mitigation

+ mapping(address => bool) public hasJoined;
​
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
​
+ if (hasJoined[msg.sender]) {
+ revert alreadyJoined();
+ }
​
// Ensure countryId is a valid index in the `teams` array
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);
+ hasJoined[msg.sender] = true;
​
numberOfParticipants++;
totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}
Updates

Appeal created

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