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

Canceled participants still counted in winner shares dilute and trap rewards

galer ah

Root + Impact

Canceled participants keep their winner share weighting, allowing griefers to dilute real winners’ payouts and permanently trap assets in the vault (High).

Description

joinEvent() permanently records each participant’s shares for their chosen country in userSharesToCountry and tracks them in usersAddress:

function joinEvent(uint256 countryId) public {
...
@> userSharesToCountry[msg.sender][countryId] = participantShares;
@> usersAddress.push(msg.sender);
...
}

When a user later cancels, the contract burns their shares and refunds stakedAsset, but it never clears their recorded country allocation:

function cancelParticipation() public {
...
stakedAsset[msg.sender] = 0;
uint256 shares = balanceOf(msg.sender);
​
@> _burn(msg.sender, shares);
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}

During setWinner, _getWinnerShares() iterates over usersAddress and sums userSharesToCountry[user][winnerCountryId] without filtering out canceled participants:

function _getWinnerShares() internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i) {
address user = usersAddress[i];
​
@> totalWinnerShares += userSharesToCountry[user][winnerCountryId];
​
}
return totalWinnerShares;
}

A malicious user can join the eventual winning country, then cancel to retrieve their stake but still remain counted in totalWinnerShares. Because finalizedVaultAsset excludes the refunded stake, each real winner’s withdrawal is diluted and leaves the vault with unusable leftover tokens.

Risk

Any participant can execute this griefing pattern: join a country, cancel before eventStartDate, and later let the true winners withdraw less. No privileged access is required, and the attack has zero net cost because the attacker recovers their entire stake.

Impact:

  • Honest winners receive only a fraction of the vault balance even though they hold all remaining shares.

  • Residual assets remain stranded in the vault forever, causing a permanent loss of funds for winners.

Proof of Concept

  1. Alice deposits 100 tokens and calls joinEvent(countryX).

  2. Bob deposits 100 tokens, calls joinEvent(countryX), then calls cancelParticipation before eventStartDate, recovering his 100 tokens while burning his shares.

  3. After the event, the owner calls setWinner(countryX). _getWinnerShares adds both Alice’s 100 shares and Bob’s stale 100 shares, setting totalWinnerShares = 200 even though only Alice’s 100 shares remain.

  4. Alice calls withdraw(). finalizedVaultAsset equals 100 (only Alice’s stake), so Alice receives Math.mulDiv(100, 100, 200) = 50 tokens while the remaining 50 tokens are trapped in the vault.

function test_griefingCanceledParticipantStillCounts() public {
// Alice joins and stays
vm.startPrank(alice);
asset.approve(address(vault), type(uint256).max);
vault.deposit(100 ether, alice);
vault.joinEvent(countryX);
vm.stopPrank();
​
// Bob joins, then cancels
vm.startPrank(bob);
asset.approve(address(vault), type(uint256).max);
vault.deposit(100 ether, bob);
vault.joinEvent(countryX);
vault.cancelParticipation();
vm.stopPrank();
​
// Event ends and winner is set
vm.warp(eventEndDate + 1);
vm.prank(owner);
vault.setWinner(countryX);
​
// Alice withdraws but receives only half of her stake
vm.prank(alice);
vault.withdraw();
assertEq(asset.balanceOf(alice), 50 ether);
}

Recommended Mitigation

  1. Track each participant’s active country and, when canceling, reset both that mapping and the user’s userSharesToCountry entry so _getWinnerShares excludes them:

function cancelParticipation() public {
...
- _burn(msg.sender, shares);
- IERC20(asset()).safeTransfer(msg.sender, refundAmount);
+ _burn(msg.sender, shares);
+ userSharesToCountry[msg.sender][userCountryId[msg.sender]] = 0;
+ IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}

  1. Consider recomputing totalWinnerShares from live share balances instead of static snapshots, or use ERC4626-style accounting so withdrawals scale only by outstanding shares.

  2. Add tests for the scenario where a participant joins, cancels, and the same country later wins to ensure they no longer affect winner accounting.

Updates

Appeal created

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