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

Incomplete State Cleanup in `cancelParticipation` Leads to Reward Dilution and Permanent Fund Lock

fawarano

Root + Impact

Description

  • normally, when a user calls cancelParticipation before the event starts, their participation should be completely removed from the system: their shares are burned, their staked assets are refunded, and all state variables tracking their participation should be reset to prevent them from affecting future reward calculations.

  • cancelParticipation function does not update key state variables (userSharesToCountry, usersAddress[], numberOfParticipants, userToCountry) that track user participation. As a result, when setWinnercalls _getWinnerShares(), it still counts cancelled users' phantom shares. This artificially inflatestotalWinnerShares`, skewing reward calculations so that legitimate winners receive less than they should and leaving part of the funds permanently locked in the contract.

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);
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
// @> Missing: userSharesToCountry[msg.sender][countryId] not reset to 0
// @> Missing: User not removed from usersAddress[] array
// @> Missing: numberOfParticipants not decremented
// @> Missing: totalParticipantShares not decremented
// @> Missing: userToCountry[msg.sender] not reset
}

Risk

Likelihood:

  • This vulnerability triggers every time a user who has already joined an event (called joinEvent) subsequently cancels their participation before the event starts

  • The impact scales linearly with the number of cancellations - each cancelled participant reduces all winners' payouts by their proportional share

Impact:

  • Direct financial loss for winners: Each legitimate winner receives a reduced payout proportional to the total phantom shares. If 50% of participants cancel, winners receive approximately 50% less than deserved

  • Permanent fund lock: The difference between what winners receive and the total vault balance remains locked forever in the contract with no recovery mechanism, as the phantom shares can never be redeemed

Proof of Concept

Add this test to test/briVault.t.sol:

POC: Proves that cancelParticipation breaks reward distribution

function test_cancelParticipation_breaks_rewards() public {
vm.prank(owner);
briVault.setCountry(countries);
​
address p1 = makeAddr("participant1");
address p2 = makeAddr("participant2");
mockToken.mint(p1, 1 ether);
mockToken.mint(p2, 1 ether);
​
// p1 deposits and joins country 10
vm.startPrank(p1);
mockToken.approve(address(briVault), 1 ether);
uint256 p1Shares = briVault.deposit(1 ether, p1);
briVault.joinEvent(10);
vm.stopPrank();
​
// p2 deposits and joins country 10
vm.startPrank(p2);
mockToken.approve(address(briVault), 1 ether);
uint256 p2Shares = briVault.deposit(1 ether, p2);
briVault.joinEvent(10);
vm.stopPrank();
​
vm.prank(p1);
briVault.cancelParticipation();
assertEq(briVault.balanceOf(p1), 0, "p1 shares should be burned");
​
uint256 vaultBalanceAfterCancel = mockToken.balanceOf(address(briVault));
console.log("Vault balance after p1 cancels:", vaultBalanceAfterCancel);
​
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(10);
​
uint256 totalWinnerShares = briVault.totalWinnerShares();
​
console.log("Total winner shares (BUGGED):", totalWinnerShares);
console.log("p2 current shares:", briVault.balanceOf(p2));
​
// BUG PROOF: totalWinnerShares includes p1's cancelled shares!
assertEq(totalWinnerShares, p1Shares + p2Shares, "totalWinnerShares includes cancelled p1 shares!");
​
uint256 p2BalanceBeforeWithdraw = mockToken.balanceOf(p2);
​
vm.prank(p2);
briVault.withdraw();
​
uint256 p2BalanceAfterWithdraw = mockToken.balanceOf(p2);
uint256 p2Payout = p2BalanceAfterWithdraw - p2BalanceBeforeWithdraw;
uint256 vaultBalanceEnd = mockToken.balanceOf(address(briVault));
console.log("Vault balance at end:", vaultBalanceEnd);
​
// Calculate what p2 SHOULD receive (all vault assets since p1 cancelled)
uint256 expectedP2Payout = vaultBalanceAfterCancel;
​
console.log("\n=== BUG IMPACT ===");
console.log("Expected p2 payout (should get ALL vault):", expectedP2Payout);
console.log("Actual p2 payout (due to bug):", p2Payout);
console.log("Funds stuck in vault forever:", vaultBalanceEnd);

Recommended Mitigation

Instead of storing all users in an array and iterating in _getWinnerShares(), maintain a running countryToTotalShares mapping updated during joinEvent() and cancelParticipation()

​
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!