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

Cancel Participation Refunds Only Latest Deposit, Stranding Prior Stakes

galer ah

Root + Impact

Description

Each call to deposit overwrites stakedAsset[receiver] with the latest net contribution (src/briVault.sol:222-223). When a participant cancels, cancelParticipation refunds only stakedAsset[msg.sender], burns all shares, and transfers that amount back (src/briVault.sol:280-288). Earlier deposits remain stranded in the vault with no shares outstanding.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
uint256 stakeAsset = assets - fee;
@> stakedAsset[receiver] = stakeAsset;
...
}
​
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);
}

Risk

Likelihood: Medium – users often top up positions before committing.

Impact:

  • Cancelling after multiple deposits refunds only the last contribution; earlier funds are permanently lost.

  • Stranded assets inflate the vault balance, skewing future share issuance.

Proof of Concept

  1. User deposits 5 ether (net ≈4.925 ether).

  2. User deposits 2 ether (net ≈1.97 ether) before the event starts.

  3. User calls cancelParticipation().

  4. Refund equals ≈1.97 ether; the earlier ≈4.925 ether stays in the vault with no shares.
    (Demonstrated in test/BriVaultAttack.t.sol:160 through testCancelOnlyRefundsLastStake.)

function testCancelOnlyRefundsLastStake() public {
briVault.deposit(FIRST_DEPOSIT, victimOne);
briVault.deposit(SECOND_DEPOSIT, victimOne);
​
uint256 balanceBefore = mockToken.balanceOf(victimOne);
briVault.cancelParticipation();
uint256 refund = mockToken.balanceOf(victimOne) - balanceBefore;
​
assertEq(refund, _netStake(SECOND_DEPOSIT)); // first stake is stranded inside the vault
}

Recommended Mitigation

  1. Accumulate stakes (stakedAsset[receiver] += stakeAsset) or track per-deposit receipts.

  2. When cancelling, return the full outstanding principal and reconcile share accounting accordingly.

  3. Add regression tests covering multi-deposit then cancel flows.

Proposed patch (Solidity-like pseudocode):

function deposit(uint256 assets, address receiver) public override returns (uint256) {
- stakedAsset[receiver] = stakeAsset;
- _mint(msg.sender, participantShares);
+ stakedAsset[receiver] += stakeAsset;
+ _mint(receiver, participantShares);
+ _afterShareChange(receiver);
}
​
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);
+ IERC20(asset()).safeTransfer(msg.sender, refundAmount);
+ _afterShareChange(msg.sender);
}
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!