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

Multiple deposits overwrite stake and burn earlier contributions

galer ah

Root + Impact

Overwriting deposits in deposit makes cancelParticipation refund only the most recent top-up, causing earlier contributions to be effectively burned and permanently trapped in the vault (High).

Description

Participants are allowed to call deposit multiple times before the event starts to increase their stake. However, the vault tracks only a single deposit slot per user and overwrites it on every call instead of accumulating the total:

function deposit(uint256 assets, address receiver) public override returns (uint256) {
...
uint256 stakeAsset = assets - fee;
​
@> stakedAsset[receiver] = stakeAsset;
​
uint256 participantShares = _convertToShares(stakeAsset);
_mint(msg.sender, participantShares);
...
}

When a participant cancels before the event starts, the contract reads stakedAsset[msg.sender] once, burns all of the user’s shares, and refunds only the last recorded value:

function cancelParticipation() public {
...
uint256 refundAmount = stakedAsset[msg.sender];
stakedAsset[msg.sender] = 0;
​
uint256 shares = balanceOf(msg.sender);
​
@> _burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}

If the user has deposited multiple times, all earlier deposits remain in the vault after _burn but are no longer claimable because the user’s shares were destroyed and their tracked stake was overwritten.

Risk

Any participant who legitimately tops up their position (for example to increase their stake or correct a previous under-deposit) and later cancels before eventStartDate will lose all but their last deposit. No special permissions are required; the bug is triggered under normal, honest user behavior.

Impact:

  • Loss of the entire value of all but the last deposit for the canceling participant.

  • Permanent increase of vault TVL without corresponding shares, skewing accounting and leaving assets stranded in the contract.

Proof of Concept

  1. Alice calls deposit(100 ether, alice) well before eventStartDate.

  2. Alice later tops up with deposit(50 ether, alice).

    • stakedAsset[alice] is overwritten from 100 ether to 50 ether.

    • Alice now owns 150 vault shares.

  3. Before the event starts, Alice calls cancelParticipation().

  4. Expected: 150 ether returned.
    Actual: only 50 ether (the last deposit) is refunded; the first 100 ether remains stuck in the vault with no shares left to claim it.

function test_cancelMultipleDepositsLosesFunds() public {
vm.startPrank(alice);
asset.approve(address(vault), type(uint256).max);
​
vault.deposit(100 ether, alice);
vault.deposit(50 ether, alice);
​
vm.warp(vault.eventStartDate() - 1);
vault.cancelParticipation();
vm.stopPrank();
​
// Alice only receives the last deposit back; earlier funds remain trapped.
assertEq(asset.balanceOf(alice), initialBalance - 50 ether);
assertEq(asset.balanceOf(address(vault)), 100 ether);
assertEq(vault.totalSupply(), 0);
}

Recommended Mitigation

  1. Track cumulative deposits per participant instead of overwriting:

- stakedAsset[receiver] = stakeAsset;
+ stakedAsset[receiver] += stakeAsset;

  1. Alternatively, derive the refund from the user’s share balance and the current asset/share ratio (for example via ERC4626-style convertToAssets) so cancelParticipation() always returns all assets backing the burned shares.

  2. Add regression tests covering multiple deposits followed by cancellation to prevent reintroducing this class of bug.

Updates

Appeal created

bryanconquer Lead Judge 16 days ago
Submission Judgement Published
Validated
Assigned finding tags:

`stakedAsset` Overwritten on Multiple Deposits

Vault tracks only a single deposit slot per user and overwrites it on every call instead of accumulating the total.

Support

FAQs

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

Give us feedback!