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

Cancelled participation does not reset internal accounting, causing reward misallocation

fredo182

Cancelled participation does not reset internal accounting, causing reward misallocation

Description

When a user cancels particiaption, their deposited funds should be fully refunded, and the vault's internal accounting should reflect that the user is no loger an active participant. This means their contribution to totalParticipantShares, numberOfParticipants, and any winner-related state should be cleared, ensuring that subsequent calculations only include active participants.

However, in the current implementation of cancelParticipation(), the function only refunds the user's staked tokens and burns their vault shares but fails to update internal state variables such as totalParticipantShares, numberOfParticipants, and userSharesToCountry. As a result, the vault continuous to treat cancelled participants as active, inflating totals and corrupting winner share calculations. This leads to incorrect winner payouts and leftover funds after withdrawals.

Risk

Likelihood: High
This occurs whenever a participant cancels after calling joinEvent().

Impact: High
Cancelled participants remain counted, inflating totals and corrupting reward distribution, causing underpayment to winners and leaving unclaimed funds locked in the vault.

Proof of Concept

The following test case provides a proof of concept in which the participant who cancelled has join the winner country.

function test_cancel_participation_after_join_event_affects_withdrawals() public {
// user1 deposits and joins a country
assertEq(mockToken.balanceOf(user1), 20 ether);
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user1);
vm.stopPrank();
assertEq(mockToken.balanceOf(user1), 19 ether);
assertEq(briVault.balanceOf(user1), 0.985 ether);
assertEq(briVault.totalAssets(), 0.985 ether);
assertEq(briVault.totalSupply(), 0.985 ether);
​
vm.prank(user1);
briVault.joinEvent(1);
assertEq(mockToken.balanceOf(user1), 19 ether);
assertEq(briVault.balanceOf(user1), 0.985 ether);
assertEq(briVault.totalAssets(), 0.985 ether);
assertEq(briVault.totalSupply(), 0.985 ether);
assertEq(briVault.totalParticipantShares(), 0.985 ether);
assertEq(briVault.numberOfParticipants(), 1);
​
// user1 cancels participation
vm.prank(user1);
briVault.cancelParticipation();
assertEq(mockToken.balanceOf(user1), 19.985 ether);
assertEq(briVault.balanceOf(user1), 0);
assertEq(briVault.totalAssets(), 0);
assertEq(briVault.totalSupply(), 0);
// Internal accounting not reset
assertEq(briVault.totalParticipantShares(), 0.985 ether); // ❌ Should be 0
assertEq(briVault.numberOfParticipants(), 1); // ❌ Should be 0
​
// Another user joins and wins — payouts are incorrect
vm.startPrank(user2);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user2);
briVault.joinEvent(1);
vm.stopPrank();
assertEq(briVault.balanceOf(user2), 0.985 ether);
assertEq(briVault.totalAssets(), 0.985 ether);
assertEq(briVault.totalSupply(), 0.985 ether);
assertEq(briVault.totalParticipantShares(), 0.985 ether * 2); // ❌ Should be shares of 1 participant
assertEq(briVault.numberOfParticipants(), 2); // ❌ Should be 1
​
vm.warp(eventEndDate + 1);
​
vm.prank(owner);
briVault.setWinner(1);
assertEq(briVault.finalizedVaultAsset(), 0.985 ether);
assertEq(briVault.totalWinnerShares(), 0.985 ether * 2); // ❌ Should be shares of 1 participant
​
vm.prank(user2);
briVault.withdraw();
assertEq(mockToken.balanceOf(user2), 19.4925 ether); // ❌ Winner underpaid
assertEq(briVault.totalAssets(), 0.4925 ether); // ❌ Leftover funds
}

Recommended Mitigation

The recommended mitigation is to clean up the protocol's state properly

function cancelParticipation() public {
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
uint256 refundAmount = stakedAsset[msg.sender];
​
stakedAsset[msg.sender] = 0;
​
uint256 shares = balanceOf(msg.sender);
​
+ // update totalParticipantShares
+ // update userSharesToCountry
+ // update numberOfParticipants
+ // update usersAddress
​
_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!