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

Cancel Participation Doesn't Clear State - Stale Mappings Remain After Cancellation

ke5haav

Root + Impact

Description

  • The cancelParticipation() function allows users to cancel their participation and receive a refund before the event starts. Under normal behavior, when a user cancels, all their participation data should be cleared, including their country selection and share mappings, ensuring a clean state if they decide to rejoin later.

  • However, the cancelParticipation() function on lines 275-289 only clears stakedAsset[msg.sender] and burns shares, but does not clear the userToCountry[msg.sender] mapping, the userSharesToCountry[msg.sender][countryId] mapping, or remove the user from the usersAddress[] array. This leaves stale data in the contract state. If a user cancels and then re-deposits and joins again, the old country mapping and share values remain, which can interfere with new participation and cause inconsistencies in winner calculations, as _getWinnerShares() will process stale share values for canceled users.

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);
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
// ❌ userToCountry[msg.sender] NOT cleared
// ❌ userSharesToCountry[msg.sender][countryId] NOT cleared
// ❌ usersAddress[] entry NOT removed
}

Risk

Likelihood:

  • This vulnerability occurs when users call cancelParticipation() and then re-deposit and join the event again, as the function does not clear the participation-related mappings before refunding

  • The bug manifests during winner calculation when _getWinnerShares() iterates through usersAddress[] and processes canceled users who still have stale share values in userSharesToCountry, causing incorrect calculations

Impact:

  • Canceled users remain in usersAddress[] array, causing _getWinnerShares() to process entries for users who are no longer participants, leading to incorrect winner share calculations

  • Stale userSharesToCountry mappings can cause canceled users' old share values to be included in totalWinnerShares calculations even though they have no shares

  • If a user cancels and rejoins, old country mappings may interfere with new participation, potentially causing confusion in winner verification

Proof of Concept

function testCancelDoesntClearState() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(10); // Join with country 10
// Cancel participation
briVault.cancelParticipation();
// Verify stale mappings remain
assertTrue(bytes(briVault.userToCountry(user1)).length > 0, "userToCountry NOT cleared");
assertGt(briVault.userSharesToCountry(user1, 10), 0, "userSharesToCountry NOT cleared");
// Verify user still in array
bool foundInArray = false;
for (uint256 i = 0; i < briVault.numberOfParticipants(); i++) {
if (briVault.usersAddress(i) == user1) foundInArray = true;
}
assertTrue(foundInArray, "User still in usersAddress array");
// User2 joins
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
briVault.joinEvent(10);
vm.stopPrank();
// Set winner and verify stale data affects calculation
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(10);
vm.stopPrank();
// totalWinnerShares includes stale shares from canceled user1
uint256 totalWinnerShares = briVault.totalWinnerShares();
uint256 user2Shares = briVault.userSharesToCountry(user2, 10);
assertGt(totalWinnerShares, user2Shares, "Stale shares from canceled user included");
}

Explanation of PoC:

This proof of concept demonstrates how stale state remains after cancellation. The test shows that when a user cancels, their mappings are not cleared, and canceled users remain in the usersAddress[] array, causing stale share values to be included in winner calculations.

Test Results:

  • ✅ userToCountry mapping not cleared after cancel

  • ✅ userSharesToCountry mapping not cleared after cancel

  • ✅ User remains in usersAddress[] array after cancel

  • ✅ Stale shares included in totalWinnerShares calculation

Recommended Mitigation

Explanation:

The recommended mitigation clears all participation-related state when a user cancels. This ensures canceled users don't affect winner calculations and users who rejoin start with clean state.

+ mapping(address => uint256) public userJoinedCountryId;
​
function joinEvent(uint256 countryId) public {
// ... existing checks ...
userToCountry[msg.sender] = teams[countryId];
+ userJoinedCountryId[msg.sender] = countryId;
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
// ... rest of function ...
}
​
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);
​
+ // Clear participation state
+ delete userToCountry[msg.sender];
+ uint256 countryId = userJoinedCountryId[msg.sender];
+ if (countryId > 0) {
+ userSharesToCountry[msg.sender][countryId] = 0;
+ delete userJoinedCountryId[msg.sender];
+ }
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
Updates

Appeal created

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