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

Cancel Participation Leaves Ghost Shares in `userSharesToCountry`

0xnaresh

Cancel Participation Leaves Ghost Shares in userSharesToCountry

Description

  • Users deposit assets and call joinEvent() to participate, storing their vault shares in userSharesToCountry[msg.sender][countryId]. The cancelParticipation() function allows users to withdraw their deposit and burn their shares before the event starts.

  • But, cancelParticipation() does not clear the entry in userSharesToCountry. When setWinner() is called, _getWinnerShares() iterates over usersAddress[] and includes canceled users' shares, inflating totalWinnerShares and diluting payouts.

// @> cancelParticipation(): burns shares but leaves stale data
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
​
// @> _getWinnerShares(): sums shares from ALL usersAddress entries
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; // ← includes canceled users
}

Risk

Likelihood:

  • Users cancel participation before event start // occurs when changing predictions or correcting mistakes

  • setWinner() is called after cancellations // standard post event flow

Impact:

  • totalWinnerShares is inflated by ghost shares // reduces payout per share

  • Legitimate winners receive less than their fair share // direct financial loss and funds stuck in vault

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
import "forge-std/Test.sol";
import "../src/briVault.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract MockAsset is ERC20 {
constructor() ERC20("Mock USDC", "USDC") {
_mint(msg.sender, 1_000_000 * 10**18);
}
}
contract BriVaultCancelGhostTest is Test {
BriVault vault;
MockAsset asset;
address owner = address(0x1);
address alice = address(0xA);
address bob = address(0xB);
address feeAddr = address(0xF);
uint256 startDate;
uint256 endDate;
function setUp() public {
asset = new MockAsset();
// fund users from test contract
asset.transfer(owner, 100_000 * 10**18);
asset.transfer(alice, 10_000 * 10**18);
asset.transfer(bob, 10_000 * 10**18);
vm.startPrank(owner);
startDate = block.timestamp + 1 days;
endDate = startDate + 7 days;
vault = new BriVault(
IERC20(asset),
0,
startDate,
feeAddr,
0,
endDate
);
string[48] memory teams;
teams[0] = "TeamA";
vault.setCountry(teams);
vm.stopPrank();
}
function testCancelLeavesGhostShares() public {
// alice joins
vm.startPrank(alice);
asset.approve(address(vault), 1000);
vault.deposit(1000, alice);
vault.joinEvent(0);
vm.stopPrank();
// alice cancels → shares burned, but ghost entry remains
vm.prank(alice);
vault.cancelParticipation();
// bob joins
vm.startPrank(bob);
asset.approve(address(vault), 1000);
vault.deposit(1000, bob);
vault.joinEvent(0);
vm.stopPrank();
// winner declared
vm.warp(endDate + 1);
vm.prank(owner);
vault.setWinner(0);
// totalWinnerShares = 2000 (alice's ghost 1000 + bob's 1000)
assertEq(vault.totalWinnerShares(), 2000);
// bob withdraws → gets only 500
uint256 bobBalBefore = asset.balanceOf(bob);
vm.prank(bob);
vault.withdraw();
uint256 payout = asset.balanceOf(bob) - bobBalBefore;
assertEq(payout, 500); // Diluted: 1000 * 1000 / 2000
// vault has 500 left (should be 0)
assertEq(asset.balanceOf(address(vault)), 500); // ← BUG: funds stuck!
}
}

Description:
Alice cancels after joining, removing her 1000 shares. Bob is the only active participant with 1000 assets in the vault. But _getWinnerShares() includes Alice’s stale shares via usersAddress[], inflating totalWinnerShares to 2000. Bob receives only 500 instead of 1000, and 500 tokens remain locked.

Recommended Mitigation

- function cancelParticipation () public {
+ 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);
+
+ // @> clear ghost shares to prevent payout inflation
+ uint256 countryId = userCountryId[msg.sender]; // requires userCountryId mapping
+ delete userSharesToCountry[msg.sender][countryId];
}
Updates

Appeal created

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