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

Users Can Join Event Multiple Times, Inflating Participant Counts

stevenalenga

Risk

Likelihood

Users calling joinEvent multiple times.

Impact

Inaccurate participant counts and totalParticipantShares, though not directly affecting payouts since totalParticipantShares is unused.

Reference Files

src/briVault.sol

Description

  • Normally, users should join the event only once to select their team, ensuring accurate statistics and preventing manipulation.

  • The issue allows users to join multiple times, incrementing participant counts and adding to totalParticipantShares each time, leading to inflated statistics. This can skew metrics and potentially allow gaming of the system.

usersAddress.push(msg.sender); // @> Pushes duplicate addresses
numberOfParticipants++;
totalParticipantShares += participantShares;

Proof of Concept

In this test, user1 joins twice, doubling the participant count incorrectly.

function testMultipleJoins() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(1); // First join: numberOfParticipants = 1
briVault.joinEvent(2); // Second join: numberOfParticipants = 2 (incorrectly inflated)
vm.stopPrank();
assertEq(briVault.numberOfParticipants(), 2); // Should be 1
}

Recommended Mitigation

Add a check to prevent multiple joins using a mapping.

+ mapping(address => bool) public hasJoined;
+
+ // In joinEvent function, add at the beginning:
+ if (hasJoined[msg.sender]) {
+ revert("Already joined");
+ }
+ hasJoined[msg.sender] = true;

totalWinnerShares Double Counted if User Joined Multiple Times

Title

totalWinnerShares Double Counted if User Joined Multiple Times

Risk

Likelihood

Users joining multiple times before winner is set.

Impact

Inflated totalWinnerShares, leading to reduced withdrawal amounts for all winners.

Reference Files

src/briVault.sol

Description

  • Normally, each participant should be counted once in calculating winner shares to ensure fair proportional payouts.

  • The problem is that usersAddress can have duplicates if users join multiple times, causing shares to be summed multiple times in _getWinnerShares, inflating totalWinnerShares and reducing individual payouts.

for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; // @> Sums for duplicates
}

Proof of Concept

User1 joins twice, leading to double-counting of shares when winner is set.

function testDoubleCountingWinnerShares() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(1); // Joins country 1
briVault.joinEvent(2); // Joins country 2, user1 in usersAddress twice
vm.stopPrank();
​
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(2); // Winner country 2, totalWinnerShares counts user1's shares twice
vm.stopPrank();
// totalWinnerShares = 10 instead of 5, reducing payouts
}

Recommended Mitigation

Ensure unique participants in usersAddress.

+ mapping(address => bool) public isParticipant;
+
+ // In joinEvent, instead of usersAddress.push(msg.sender);
+ if (!isParticipant[msg.sender]) {
+ usersAddress.push(msg.sender);
+ isParticipant[msg.sender] = true;
+ }
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!