BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

Missing State Validation in cancelParticipation() Allows Redundant Execution and Accounting Inconsistency

sidd

Root + Impact

Description

The cancelParticipation() function allows users to withdraw their deposited funds before the event begins. However, the contract does not validate whether the caller has already cancelled or previously withdrawn their stake. Because there is no guard preventing repeated calls, the same user can invoke cancelParticipation() multiple times before the event start date.

While stakedAsset[msg.sender] is reset to 0 after the first call, the function continues to execute successfully in subsequent calls — performing a _burn() operation (even if balanceOf(msg.sender) is already zero) and emitting no error. This behavior leads to unnecessary gas consumption, inconsistent accounting, and potential logical drift between share supply and participant tracking.

If the _burn() is called multiple times on an address with no remaining shares, it may desynchronize total share supply, impacting later calculations in functions like setWinner() and withdraw().

function cancelParticipation() public {
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
@> uint256 refundAmount = stakedAsset[msg.sender]; // Reads current user stake
@> stakedAsset[msg.sender] = 0; // Resets stake, but does not block repeated calls
​
uint256 shares = balanceOf(msg.sender);
@> _burn(msg.sender, shares); // Can burn 0 repeatedly, causing total supply inconsistencies
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}

Risk

Likelihood:

  • The issue can be triggered both unintentionally (user double-clicks cancel in UI) or intentionally (malicious scripts repeatedly calling before eventStartDate).

  • No state-based restriction prevents redundant executions.

Impact:

  • Causes incorrect total share supply and potential inconsistencies in later settlement calculations.

  • May open paths for griefing or denial-of-service if mass redundant cancellations occur, bloating storage and wasting gas.

Proof of Concept

function test_multipleCancelParticipation() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
​
// Step 1: Deposit normally
briVault.deposit(5 ether, user1);
console.log("=== AFTER DEPOSIT ===");
console.log("balanceOf(user1):", briVault.balanceOf(user1));
console.log("stakedAsset(user1):", briVault.stakedAsset(user1));
​
// Step 2: First cancel — expected
briVault.cancelParticipation();
console.log("=== AFTER FIRST CANCEL ===");
console.log("stakedAsset(user1):", briVault.stakedAsset(user1));
console.log("balanceOf(user1):", briVault.balanceOf(user1));
​
// Step 3: Second cancel — should revert but executes again
briVault.cancelParticipation();
console.log("=== AFTER SECOND CANCEL (VULNERABILITY) ===");
console.log("stakedAsset(user1):", briVault.stakedAsset(user1));
console.log("balanceOf(user1):", briVault.balanceOf(user1));
​
vm.stopPrank();
console.log("=== MULTIPLE-CANCEL-DEBUG-END ===");
}

Observed Output

=== AFTER DEPOSIT ===
balanceOf(user1): 4925000000000000000
stakedAsset(user1): 4925000000000000000
​
=== AFTER FIRST CANCEL ===
stakedAsset(user1): 0
balanceOf(user1): 0
​
=== AFTER SECOND CANCEL (VULNERABILITY) ===
stakedAsset(user1): 0
balanceOf(user1): 0

The test demonstrates that cancelParticipation() can be called multiple times without reverting, even after the user’s stake and shares have been fully reset.
While the second call doesn’t transfer additional funds (since stakedAsset is 0), it still executes _burn() and safeTransfer() logic, silently allowing redundant execution.
This proves the function lacks a state validation guard, creating potential for gas griefing, accounting drift, and inconsistent event participation logic.

Recommended Mitigation

@@
function cancelParticipation() public {
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
+ // Prevent redundant cancellations — require an existing stake
+ if (stakedAsset[msg.sender] == 0) {
+ revert noDeposit();
+ }
+
uint256 refundAmount = stakedAsset[msg.sender];
stakedAsset[msg.sender] = 0;
-
- uint256 shares = balanceOf(msg.sender);
-
- _burn(msg.sender, shares);
-
- IERC20(asset()).safeTransfer(msg.sender, refundAmount);
+ // burn only if user actually has shares (defensive)
+ uint256 shares = balanceOf(msg.sender);
+ if (shares > 0) {
+ _burn(msg.sender, shares);
+ }
+
+ IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}

Added a guard that reverts when stakedAsset[msg.sender] == 0, blocking repeated cancelParticipation() calls after the first cancellation. Also burn shares only when balanceOf(msg.sender) > 0 to avoid redundant _burn() calls. These minimal edits prevent redundant refunds, avoid repeated/meaningless burns, and keep vault accounting consistent with the intended single-cancel flow.

Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!