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

cancelParticipation() Doesn't Reset joinEvent() State

icis

cancelParticipation() Doesn't Reset joinEvent() State Causing Direct Financial Loss to Legitimate Winners

Description

  • Expected beahviour: when a user cancels participation, the following is meant to happen:

    • When user cancels, stakedAsset is reset and shares are burned

    • userToCountry[msg.sender] is cleared

    • userSharesToCountry[msg.sender][countryId] is cleared

    • User is removed from usersAddress[]

    • numberOfParticipants is decremented

    • totalParticipantShares is decremented

  • Actual Behaviour:

    • When user cancels, stakedAsset is reset and shares are burned

    • BUT userToCountry[msg.sender] is NOT cleared

    • userSharesToCountry[msg.sender][countryId] is NOT cleared

    • User is NOT removed from usersAddress[]

    • numberOfParticipants is NOT decremented

    • totalParticipantShares is NOT decremented

Location: cancelParticipation() function (lines 275-289)

function cancelParticipation () public {
// ...
stakedAsset[msg.sender] = 0;
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
// ...
}

Risk

Likelihood: High: natural users may cancel participation for any reason leading to the vulnerability

Impact:

  • If user cancels then deposits again and joins different country:

    • Old country selection remains in mappings

    • User appears in usersAddress[] twice (if they rejoin)

    • _getWinnerShares() may count old country selection

  • Inflated participant counts

  • Incorrect winner share calculations

Proof of Concept

Example:

  1. Alice deposits 10 ether, gets 9.85 shares, joins country 10

  1. Bob deposits 10 ether, gets 9.85 shares, joins country 10

  1. Alice cancels (does not rejoin) → shares burned, but stale data remains

  1. Country 10 wins

Results:

  • Expected winner shares: 9.85 (only Bob)

  • Actual winner shares: 19.7 (Bob + Alice's stale 9.85)

  • Inflation: 100% (doubled)

  • Bob's expected withdrawal: 9.85 ether (100% of vault)

  • Bob's actual withdrawal: 4.925 ether (50% of vault)

  • Bob's loss: 4.925 ether (50% loss)

  • Locked funds: 4.925 ether (permanently stuck)

// Add this to the test suite
address owner = makeAddr("owner");
address alice = makeAddr("alice");
​
function test_FinancialImpact() public {
// Setup: Alice and Bob both deposit and join country 10
vm.startPrank(alice);
mockToken.approve(address(briVault), 10 ether);
uint256 aliceShares = briVault.deposit(10 ether, alice);
briVault.joinEvent(10);
vm.stopPrank();
vm.startPrank(bob);
mockToken.approve(address(briVault), 10 ether);
uint256 bobShares = briVault.deposit(10 ether, bob);
briVault.joinEvent(10);
vm.stopPrank();
// Alice cancels (does NOT rejoin)
vm.startPrank(alice);
briVault.cancelParticipation();
vm.stopPrank();
// Verify stale data persists
uint256 staleShares = briVault.userSharesToCountry(alice, 10);
assertEq(briVault.balanceOf(alice), 0, "Alice has 0 shares");
assertGt(staleShares, 0, "Stale shares remain");
console.log("Stale shares (BUG):", staleShares);
// Event ends, country 10 wins
vm.warp(block.timestamp + 35 days);
vm.prank(owner);
briVault.setWinner(10);
uint256 finalizedVault = briVault.finalizedVaultAsset();
uint256 totalWinnerShares = briVault.totalWinnerShares();
uint256 expectedWinnerShares = bobShares;
console.log("Expected winner shares:", expectedWinnerShares);
console.log("Actual winner shares (inflated):", totalWinnerShares);
console.log("Inflation %:", ((totalWinnerShares - expectedWinnerShares) * 100) / expectedWinnerShares);
assertEq(totalWinnerShares, bobShares + aliceShares, "Includes stale shares");
// Bob withdraws
uint256 bobBalanceBefore = mockToken.balanceOf(bob);
vm.prank(bob);
briVault.withdraw();
uint256 bobReceived = mockToken.balanceOf(bob) - bobBalanceBefore;
uint256 bobExpected = finalizedVault;
uint256 bobActual = (bobShares * finalizedVault) / totalWinnerShares;
console.log("Bob expected:", bobExpected);
console.log("Bob received:", bobReceived);
console.log("Bob loss %:", ((bobExpected - bobReceived) * 100) / bobExpected);
console.log("Locked funds:", mockToken.balanceOf(address(briVault)));
assertEq(bobReceived, bobActual, "Bob received reduced amount");
assertLt(bobReceived, bobExpected, "Bob lost funds due to stale data");
assertGt(mockToken.balanceOf(address(briVault)), 0, "Funds locked in vault");
}
​

Recommended Mitigation

clear joinEvent stae and clear all mappings for the user

function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
​
uint256 refundAmount = stakedAsset[msg.sender];
stakedAsset[msg.sender] = 0;
// Clear joinEvent state
delete userToCountry[msg.sender];
// Clear all country mappings for user
for (uint256 i = 0; i < teams.length; ++i) {
delete userSharesToCountry[msg.sender][i];
}
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!