BriVault

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

cancelParticipation() Does Not Clear User Country Shares — Locked Shares Remain in Contract and Winners get Wrong Rewards

Root + Impact

Description

  • Normal behavior:
    When a user cancels participation before the event starts, their deposited assets are refunded, their vault shares are burned, and all related accounting should be updated so that their participation no longer affects totals or winner calculations.

  • Issue:
    Currently, cancelParticipation() refunds assets and burns the user’s ERC20 shares, but does not reset userSharesToCountry mapping. The user’s shares remain associated with their country in the userSharesToCountry mapping. This causes:

    • Shares to remain effectively “stuck” in the contract.

    • Distorted calculations of totalWinnerShares and eventual payout distributions.

    • Users who canceled can reduce rewards for other participants unintentionally.

function cancelParticipation() public {
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
uint256 refundAmount = stakedAsset[msg.sender];
stakedAsset[msg.sender] = 0;
@> totalParticipantShares -= balanceOf(msg.sender);
@> uint256 shares = balanceOf(msg.sender);
@> _burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
@> // Missing: clearing userSharesToCountry for the canceled user's country
}

Risk

Likelihood:

  • Occurs whenever a participant cancels after joining an event and before it starts.

  • Particularly relevant in multi-user events where some participants withdraw early.

Impact:

  • Remaining users receive incorrect or reduced rewards, since the canceled user’s shares are still counted in userSharesToCountry.

  • Locked shares remain in the contract, making assets partially unrecoverable.

  • Misleading accounting for totalWinnerShares and overall vault state.


Proof of Concept

  • This PoC shows that when a user cancels participation, their deposit is refunded and shares are burned, but their userSharesToCountry entry remains unchanged. As a result, those shares are still counted in global calculations, causing leftover assets to remain locked in the contract and reducing rewards for remaining participants. *

function testCancelEventDoesntReduceShares() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
vm.stopPrank();
vm.startPrank(user1);
briVault.joinEvent(1);
vm.stopPrank();
vm.startPrank(user2);
briVault.joinEvent(1);
vm.stopPrank();
vm.startPrank(user1);
briVault.cancelParticipation();
vm.stopPrank();
vm.startPrank(owner);
vm.warp(eventEndDate + 1);
briVault.setWinner(1);
vm.stopPrank();
// the user 2 has withdrawn the amount, technically the user should have got all the money,
// but due to not clearing in userToSharesCountry, the user got wrong amount and remaining amount got stuck in contract
vm.startPrank(user2);
briVault.withdraw();
vm.stopPrank();
// The vault still holds shares from user1 in userSharesToCountry, locking assets
assertEq(
mockToken.balanceOf(address(briVault)),
4925000000000000000 / 2
);
}

Recommended Mitigation

Introduce a mapping userToCountryId to track the country ID for each user. When user join event, store the countryID of user and When a user cancels participation, clear their shares from userSharesToCountry using this mapping.

function joinEvent(uint256 countryId) public {
/// remaining same code
+ userToCountry[msg.sender] = teams[countryId];
}
/// FUNCTION 2
function cancelParticipation() public {
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
uint256 refundAmount = stakedAsset[msg.sender];
stakedAsset[msg.sender] = 0;
totalParticipantShares -= balanceOf(msg.sender);
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
+ // Clear the canceled user's country-specific shares
+ userSharesToCountry[msg.sender][userToCountryId[msg.sender]] = 0;
}

With this fix, canceled users no longer contribute to country-specific or total shares, preventing locked funds and ensuring correct reward distributions for remaining participants.

Updates

Appeal created

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