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

cancelParticipation() Does Not Update totalParticipantShares — Misrepresenting Total Shares After User Withdrawal and numberOfParticipants

0xzulkefal

Root + Impact

Description

  • Normal behavior:
    When a user cancels participation, their deposited assets are refunded and their vault shares are burned. The system should also update all related global accounting variables, including totalParticipantShares , numberOfParticipants and per-country user shares, to accurately reflect the current state.

  • Issue:
    The current cancelParticipation() implementation only burns the user’s ERC20 shares and refunds the staked asset. It does not adjust the totalParticipantShares or the userSharesToCountry mapping. This causes misrepresentation of total shares: the contract still counts shares for a user who has exited, leading to potential distortions in reward calculations, winner distributions, and overall vault accounting.

function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
​
uint256 refundAmount = stakedAsset[msg.sender];
​
@> stakedAsset[msg.sender] = 0;
​
uint256 shares = balanceOf(msg.sender);
​
@> _burn(msg.sender, shares); // user shares removed from ERC20, but not from totalParticipantShares or userSharesToCountry
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
​

Risk

Likelihood:

  • Occurs every time a participant cancels after joining an event.

  • Particularly likely in events with multiple participants where some users opt to withdraw early.

Impact:

  • Total participant shares are inflated, giving a false picture of overall participation.

  • Winners’ reward calculations based on totalWinnerShares will be incorrect, potentially reducing payouts for legitimate winners.

  • Misrepresented state may also confuse front-end dashboards or auditing tools, creating a perception of incorrect accounting.

Proof of Concept

This PoC demonstrates that after a user cancels participation, totalParticipantShares does not decrease, even though the user no longer holds shares. This creates inaccurate accounting for the vault.

function testTotalSharesAfterCancelParticipation() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
vm.stopPrank();
​
briVault.totalAssetsShares();
​
assertEq(mockToken.balanceOf(address(participationFeeAddress)), 0.075 ether);
​
vm.startPrank(user1);
briVault.joinEvent(1);
uint256 totalShares = briVault.totalParticipantShares();
briVault.userToCountry(user1);
briVault.stakedAsset(user1);
briVault.userSharesToCountry(user1, 1);
vm.stopPrank();
​
vm.startPrank(user1);
briVault.cancelParticipation();
​
// totalParticipantShares remains unchanged, leading to misrepresentation
assertEq(totalShares, briVault.totalParticipantShares());
vm.stopPrank();
}
​

Recommended Mitigation

Update cancelParticipation() to properly remove the user’s shares from totalParticipantShares

​
function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
​
uint256 refundAmount = stakedAsset[msg.sender];
​
stakedAsset[msg.sender] = 0;
​
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
+ totalParticipantShares -= balanceOf(msg.sender);
​
+ numberOfParticipants--;
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
​
​
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!