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

Incomplete State Cleanup in cancelParticipation() Leads to Incorrect Winner Calculations

objectplayer

Root + Impact

Incomplete state cleanup when users cancel participation leads to incorrect winner share calculations and potential fund loss.

Description

  • When a user cancels their participation, all related state variables should be cleared to reflect they are no longer participating in the event.

  • The cancelParticipation() function only resets stakedAsset[msg.sender] and burns shares, but fails to clean up critical state variables including userToCountry, userSharesToCountry, and the user's entry in usersAddress array. It also fails to decrement numberOfParticipants and totalParticipantShares.

function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
​
uint256 refundAmount = stakedAsset[msg.sender];
​
stakedAsset[msg.sender] = 0; // Only this is cleared
​
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
// @> Missing cleanup:
// @> - userToCountry[msg.sender] not deleted
// @> - userSharesToCountry[msg.sender][countryId] not deleted
// @> - usersAddress still contains the user
// @> - numberOfParticipants not decremented
// @> - totalParticipantShares not decremented
}

Risk

Likelihood:

  • Occurs every single time a user calls cancelParticipation()

  • This is normal user behavior, not an edge case

  • State corruption accumulates with each cancellation

Impact:

  • _getWinnerShares() will count cancelled users' shares in calculation

  • totalWinnerShares becomes inflated with burned/cancelled shares

  • Winners receive significantly less funds than deserved

  • numberOfParticipants becomes permanently incorrect

  • User can rejoin and appear multiple times in usersAddress

  • Direct fund loss for legitimate winners

Proof of Concept

Here is the PoC where a user deposit some amount, join the event and cancel the event. But user amount is still counted in the shares

// User1 deposits 1000 tokens and joins country 0
vault.deposit(1000e18, user1);
vault.joinEvent(0);
// usersAddress = [user1]
// userSharesToCountry[user1][0] = 100 shares
// numberOfParticipants = 1
​
// User1 cancels participation
vault.cancelParticipation();
// Refund received, shares burned
// BUT state persists:
// usersAddress = [user1] (still there!)
// userToCountry[user1] = teams[0] (still there!)
// userSharesToCountry[user1][0] = 100 (still there!)
// numberOfParticipants = 1 (should be 0!)
​
// Owner sets country 0 as winner
vault.setWinner(0);
// _getWinnerShares() loops through usersAddress
// Adds user1's shares: totalWinnerShares += 100
// But user1's shares were already burned!
​
// LegitimateWinner tries to withdraw
vault.withdraw();
// Calculation: (legitimateShares * vaultAsset) / (totalWinnerShares + 100)
// Gets less than deserved due to inflated denominator!

Recommended Mitigation

Here is the simple mitigation to fix this issue

function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
​
uint256 refundAmount = stakedAsset[msg.sender];
+ uint256 shares = balanceOf(msg.sender);
+
+ // Get user's country to clean up mappings
+ string memory userCountry = userToCountry[msg.sender];
+ uint256 countryId;
+ for (uint256 i = 0; i < teams.length; i++) {
+ if (keccak256(abi.encodePacked(teams[i])) == keccak256(abi.encodePacked(userCountry))) {
+ countryId = i;
+ break;
+ }
+ }
​
stakedAsset[msg.sender] = 0;
+ delete userToCountry[msg.sender];
+ delete userSharesToCountry[msg.sender][countryId];
+
+ // Remove from usersAddress array
+ 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;
​
- 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!