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

Canceled Participants’ Stale Winner Shares Inflate Denominator and Lock Funds in Vault

themilenkov

The cancelParticipation() function burns a user’s vault shares and refunds their deposit, but does not clear their recorded userSharesToCountry entry, this causes legitimate winners to receive a smaller payout than they deserve.

Description

When a participant cancels before the event starts, their deposited stake is refunded and their ERC4626 shares are burned. However, the user’s recorded participation data (userSharesToCountry and usersAddress) is not cleared or updated.

Later, during setWinner(), the contract calls _getWinnerShares() which aggregates all users’ shares from userSharesToCountry to compute totalWinnerShares. Because canceled users remain in this mapping with their old share values, their “ghost shares” are still included in the total. This artificially inflates the denominator used in the withdraw() payout calculation, reducing the payout for legitimate winners and leaving residual assets stranded in the vault.

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);
}

Risk

Likelihood:

  • Any normal user cancelling their participation triggers the condition. No special privileges or attack sequence are needed.

Impact:

  • Incorrect accounting leads to:

    • Miscalculated totalWinnerShares

    • Reduced withdrawals for valid winners

    • Permanent funds are locked in the vault

    The bug directly undermines the fairness and correctness of the reward distribution mechanism.

Proof of Concept

Add the following code to briVault.t.sol

/// @notice Demonstrates that cancelParticipation() leaves stale winner shares
/// in accounting, causing the sole real winner to receive less than
/// the full finalized vault balance and leaving funds stuck in the vault.
function testGhostSharesFromCancelledParticipantReduceWinnerPayout() public {
// Owner sets countries so joinEvent & setWinner work
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
// === Setup: two users pick the same country, one later cancels ===
​
// user1 deposits and joins country 0 (e.g. "United States")
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(0);
vm.stopPrank();
​
// user2 deposits and joins the SAME winning country 0
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
briVault.joinEvent(0);
​
// user2 cancels participation BEFORE eventStartDate
briVault.cancelParticipation();
vm.stopPrank();
​
// At this point:
// - user1 has shares and is registered to country 0
// - user2 has 0 shares, but userSharesToCountry[user2][0] is STILL > 0
​
// === End the event and set winner ===
vm.warp(eventEndDate + 1);
​
vm.startPrank(owner);
briVault.setWinner(0); // country 0 is declared the winner
uint256 finalized = briVault.finalizedVaultAsset();
vm.stopPrank();
​
// Sanity: snapshot should equal vault balance at this moment
assertEq(finalized, mockToken.balanceOf(address(briVault)));
​
// === Winner (user1) withdraws ===
uint256 user1Before = mockToken.balanceOf(user1);
uint256 vaultBefore = mockToken.balanceOf(address(briVault));
​
vm.startPrank(user1);
briVault.withdraw();
vm.stopPrank();
​
uint256 user1After = mockToken.balanceOf(user1);
uint256 vaultAfter = mockToken.balanceOf(address(briVault));
uint256 user1Payout = user1After - user1Before;
​
// No winner with positive shares remains:
assertEq(briVault.balanceOf(user1), 0);
assertEq(briVault.balanceOf(user2), 0);
​
// Under correct logic, user1 is the ONLY real winner with shares,
// so they should receive the entire finalizedVaultAsset.
// Due to ghost shares from user2, user1 gets strictly less than `finalized`
// and some funds remain stuck in the vault.
assertLt(user1Payout, finalized);
assertGt(vaultAfter, 0);
​
// Conservation: snapshot = paid to winner + leftover in vault
assertEq(user1Payout + vaultAfter, finalized);
​
// This confirms that the stale userSharesToCountry entry for user2
// inflated totalWinnerShares and reduced user1's payout.
}

Recommended Mitigation

function cancelParticipation() public {
if (block.timestamp >= eventStartDate) revert eventStarted();
uint256 refundAmount = stakedAsset[msg.sender];
stakedAsset[msg.sender] = 0;
​
+ // Clear participation data
+ string memory country = userToCountry[msg.sender];
+ if (bytes(country).length > 0) {
+ for (uint256 i; i < teams.length; ++i) {
+ if (keccak256(bytes(teams[i])) == keccak256(bytes(country))) {
+ userSharesToCountry[msg.sender][i] = 0;
+ break;
+ }
+ }
+ }
+ userToCountry[msg.sender] = "";
​
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
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!