BriVault

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

Users Can Repeatedly Join Event, Allowing Team Switching and State Corruption

Root + Impact

Description

  • The intended behavior is that a user deposits funds and then calls joinEvent() once to lock in their prediction for the tournament winner.

    The problem is that the joinEvent() function has no mechanism to check if a user (msg.sender) has already joined. This oversight allows a single user to call the function multiple times before the event starts. Each subsequent call overwrites their previous team selection and, more critically, adds their address as a duplicate entry to the usersAddress array and incorrectly increments the numberOfParticipants counter.

function joinEvent(uint26 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
// ...
@> // MISTAKE: No check to see if the user has already joined.
@> userToCountry[msg.sender] = teams[countryId]; // Overwrites previous selection.
uint26 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
@> usersAddress.push(msg.sender); // Adds a duplicate address to the array.
@> numberOfParticipants++; // Incorrectly inflates the participant count.
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}

Risk

Likelihood:

  • The function is public and unprotected. Any user who has deposited can call it multiple times.

Impact:

  • Unfair Team Switching: Users can change their bet at any time before the event starts, potentially reacting to new information in a way that should be disallowed.

  • State Corruption: The numberOfParticipants becomes an inaccurate count of total joins rather than unique participants. The usersAddress array contains duplicate data.

  • Exacerbates a Critical Vulnerability: Each repeated call adds to the usersAddress array, making the unbounded loop in _getWinnerShares() more expensive and bringing the contract closer to the gas limit that triggers the permanent Denial of Service.

Proof of Concept

  • The test first establishes a baseline by having a user legitimately deposit funds and join the event, betting on "Brazil." It then demonstrates the exploit by having the same user call joinEvent a second time to switch their bet to "Japan." The test concludes by confirming the negative impacts: the user unfairly changed their team, and the contract's internal accounting is now corrupt, showing two participants instead of one, which worsens a more critical Denial of Service vulnerability.

function test_POC_UserCanJoinMultipleTimes() public {
// Step 1: Owner sets up the countries.
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
// Step 2: User1 deposits 1 ether.
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user1);
// Step 3: User1 joins for the first time, betting on "Brazil" (index 4).
briVault.joinEvent(4);
// Verify initial state is correct.
assertEq(briVault.numberOfParticipants(), 1, "Participant count should be 1");
assertEq(briVault.userToCountry(user1), "Brazil", "Initial team selection should be Brazil");
// Step 4: THE EXPLOIT - User1 joins a second time, changing their bet to "Japan" (index 10).
briVault.joinEvent(10);
vm.stopPrank();
assertEq(briVault.userToCountry(user1), "Japan", "User should have been able to switch team to Japan");
assertEq(briVault.numberOfParticipants(), 2, "BUG: Participant count is incorrectly inflated to 2");
console2.log("Corrupted Participant Count:", briVault.numberOfParticipants());
}

Recommended Mitigation

  • To prevent this, add a state-tracking mechanism, such as a mapping, to record whether a user has already joined and enforce a one-time participation rule.

// File: src/briVault.sol
contract BriVault is ERC4626, Ownable {
// ... other state variables ...
address[] public usersAddress;
+ mapping(address => bool) public hasUserJoined;
// ... constructor and other functions ...
function joinEvent(uint26 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
+
+ // Add check to ensure user can only join once.
+ require(!hasUserJoined[msg.sender], "BriVault: You have already joined the event.");
// ... other checks and logic ...
userToCountry[msg.sender] = teams[countryId];
uint26 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
usersAddress.push(msg.sender);
+ hasUserJoined[msg.sender] = true; // Mark the user as having joined.
numberOfParticipants++;
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}```
Updates

Appeal created

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