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

cancelParticipation() Does Not Decrement `numberOfParticipants`

aziz0x48

Summary

Users deposit tokens and call joinEvent() to register for a country. The numberOfParticipants counter is incremented on every joinEvent() call to track total participants.

When a user calls cancelParticipation() to withdraw their deposit before the event starts, the counter is not decremented, leaving numberOfParticipants permanently inflated.

Description

function joinEvent(uint256 countryId) public {
// ...
usersAddress.push(msg.sender);
numberOfParticipants++; // @> incremented on every call
// ...
}
​
function cancelParticipation() public {
// ...
stakedAsset[msg.sender] = 0;
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
// @> missing: numberOfParticipants--;
}

Risk

Likelihood: Medium

  • Users who deposit and later cancel participation trigger the inconsistency.

Impact: Low

  • numberOfParticipants becomes inaccurate and misleading.

  • UI or future logic added that is relying on this value will be incorrect.

Proof of Concept

Add the following test function to BriVaultTest.t.sol and run forge test --mt testCancelDoesNotDecrementParticipants

function testCancelDoesNotDecrementParticipants() public {
// user1 deposits and joins an event
vm.prank(user1);
mockToken.approve(address(briVault), type(uint256).max);
vm.prank(user1);
briVault.deposit(1 ether, user1);
vm.prank(user1);
briVault.joinEvent(0);
//numberOfParticipants value should be 1
assertEq(briVault.numberOfParticipants(), 1);
// user 1 cancels his participation
vm.prank(user1);
briVault.cancelParticipation();
​
//numberOfParticipants value should be 0, but it will revert because its still 1
assertEq(briVault.numberOfParticipants(), 0);
}

Recommended Mitigation

Simply decrement the value of numberOfParticipantsafter the user calls cancelParticipation()

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);
​
+ // Decrement participant count
+ numberOfParticipants--;
}
Updates

Appeal created

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