BriVault

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

BriVault::cancelParticipation() does not update internal contract records, leading to stale participant data and broken accounting

BriVault::cancelParticipation() does not update internal contract records, leading to stale participant data and broken accounting

Description

  • The BriVault::cancelParticipation() function likely intends to allow users to “withdraw” or “opt out” of an event.

  • But, it does not remove the user’s address from the usersAddress array, nor does it decrement numberOfParticipants or reset related mappings such as userToCountry or userSharesToCountry.

  • Relevant vulnerable code section:

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);
}

Risk

Likelihood:

  • Any user can trigger this behavior by calling cancelParticipation() after joining.

Impact:

  • usersAddress retains users who have already cancelled, producing stale data.

  • numberOfParticipants remains inflated, breaking metrics or share-based calculations.

  • totalParticipantShares may also remain inaccurate if not decremented.

  • Subsequent on-chain queries (e.g., for leaderboard or reward distribution) produce incorrect results.

Proof of Concept

The following test demonstrates the issue clearly:

function testCancelParticipationNotUpdatesContractRecord() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(40);
briVault.cancelParticipation();
vm.stopPrank();
console.log(briVault.usersAddress(0)); // ❌ still user1
console.log(user1);
console.log(briVault.numberOfParticipants()); // ❌ still 1
assert(briVault.usersAddress(0) == user1);
assert(briVault.numberOfParticipants() == 1);
}

Observed behavior:

  • After cancelParticipation(), user1 still appears as the first element in usersAddress.

  • numberOfParticipants remains 1.

  • The internal contract state suggests that the user is still participating.

Expected behavior:

  • Once a user cancels, they should be removed from usersAddress, numberOfParticipants should decrement by 1, and all user-related mappings (userToCountry, userSharesToCountry, etc.) should be reset.

Recommended Mitigation

Implement full participant record clean-up when cancelling participation.

+ event participationCancelled(address user);
...
function cancelParticipation() public {
+ if (stakedAsset[msg.sender] == 0) revert noDeposit();
if (block.timestamp > eventStartDate) revert eventStarted();
uint256 refundAmount = stakedAsset[msg.sender];
// Handle refund or unstake logic here
stakedAsset[msg.sender] = 0;
+ // ✅ Clean up user-related mappings
+ delete userToCountry[msg.sender];
+ delete userSharesToCountry[msg.sender];
+ hasJoined[msg.sender] = false;
+ // ✅ Remove user 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;
+ }
+ }
+ // ✅ Update participant counters
+ if (numberOfParticipants > 0) {
+ numberOfParticipants--;
+ }
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
+ emit participationCancelled(msg.sender);
}

This ensures the contract’s state remains synchronized and accurately reflects active participants.

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!