Submission Details
Severity:
cancelParticipation() Refunds Only Last Deposit, Burns All Shares
Root + Impact
Description
BriVault's deposit() function overwrites stakedAsset[receiver] instead of accumulating deposits. When users make multiple deposits then call cancelParticipation(), the function burns all accumulated shares but refunds only the last deposit amount, causing permanent loss of all prior deposits.
Root cause: BriVault's deposit() function overwrites stakedAsset[receiver] instead of accumulating deposits.
function deposit(uint256 assets, address receiver) public override returns (uint256) {
// ... fee calculation ...
@> stakedAsset[receiver] = stakeAsset; // PROBLEM: OVERWRITES instead of accumulating!
// ... rest of function ...
}
Risk
Likelihood: High
Multiple deposits followed by cancelParticipation is legitimate user behavior
Impact: High
Users lose all deposits except the last one when canceling participation
Protocol burns all accumulated shares but refunds only last deposit amount
Trust destruction when users discover fund losses from legitimate behavior
Tournament participation becomes irrational for multi-deposit scenarios
Proof of Concept
The POC demonstrates how cancelParticipation() burns all accumulated shares but only refunds the last deposit amount due to deposit() overwriting stakedAsset instead of accumulating. It shows a user making multiple deposits, accumulating shares, then calling cancelParticipation() which burns all shares but refunds only the final deposit amount. The test verifies that all prior deposits are permanently lost, creating a critical accounting bug in the cancellation mechanism.
// SPDX-License-Identifier: MIT
import "forge-std/Test.sol";
import {BriVault} from "../../src/briVault.sol";
import {ERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
/**
* PoC: cancelParticipation() Refunds Only Last Deposit, Burns All Shares
* Discovery Method: Accounting Logic Analysis
* Severity: HIGH
* Impact: permanent user fund loss
*
* Root Cause: stakedAsset mapping overwritten on deposit, cancelParticipation burns all shares
* but refunds only last deposit amount, causing users to lose all prior deposits permanently.
*/
contract MockERC20 is ERC20 {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}
contract CancelParticipationAccountingBugPoC is Test {
BriVault vault;
MockERC20 asset;
address owner = makeAddr("owner");
address victim = makeAddr("victim");
address feeRecipient = makeAddr("feeRecipient");
uint256 constant PARTICIPATION_FEE_BPS = 100; // 1%
uint256 constant MINIMUM_AMOUNT = 100 ether;
uint256 EVENT_START;
uint256 EVENT_END;
function setUp() public {
// Initialize time variables
EVENT_START = block.timestamp + 1 days;
EVENT_END = EVENT_START + 7 days;
// Deploy mock ERC20 asset
asset = new MockERC20("Mock Token", "MOCK");
// Deploy vault with tournament parameters
vm.startPrank(owner);
vault = new BriVault(
IERC20(address(asset)),
PARTICIPATION_FEE_BPS,
EVENT_START,
feeRecipient,
MINIMUM_AMOUNT,
EVENT_END
);
// Set up tournament countries
string[48] memory countries;
countries[0] = "Brazil";
countries[1] = "Argentina";
vault.setCountry(countries);
vm.stopPrank();
// Setup victim balance
asset.mint(victim, 1000 ether);
// Approve vault to spend tokens
vm.startPrank(victim);
asset.approve(address(vault), type(uint256).max);
vm.stopPrank();
}
/**
* CRITICAL VULNERABILITY: cancelParticipation() accounting bug causes permanent fund loss
*
* Attack Flow:
* 1. User makes multiple deposits legitimately (each overwrites stakedAsset)
* 2. User calls cancelParticipation() before event starts
* 3. Function burns all accumulated shares but refunds only last deposit amount
* 4. User permanently loses all prior deposits
*/
function test_CancelParticipationAccountingBug() public {
console.log("=== CANCEL PARTICIPATION ACCOUNTING BUG ===");
// Phase 1: Victim makes multiple legitimate deposits
vm.startPrank(victim);
// First deposit: 200 ether - pays 2 ether fee, stakes 198 ether
vm.warp(EVENT_START - 1 hours);
vault.deposit(200 ether, victim);
console.log("First deposit: 200 ether, stakedAsset =", vault.stakedAsset(victim));
// Second deposit: 200 ether - pays 2 ether fee, stakes 198 ether
vault.deposit(200 ether, victim);
console.log("Second deposit: 200 ether, stakedAsset =", vault.stakedAsset(victim));
console.log("Notice: stakedAsset overwritten, lost track of first 198 ether!");
// Third deposit: 200 ether - pays 2 ether fee, stakes 198 ether
vault.deposit(200 ether, victim);
console.log("Third deposit: 200 ether, stakedAsset =", vault.stakedAsset(victim));
console.log("Total deposited: 600 ether, total fees: 6 ether");
console.log("Expected refund on cancel: 594 ether");
console.log("But stakedAsset only tracks: 198 ether (last deposit)");
vm.stopPrank();
// Phase 2: Victim cancels participation
vm.startPrank(victim);
uint256 balanceBeforeCancel = asset.balanceOf(victim);
console.log("Balance before cancelParticipation():", balanceBeforeCancel);
vault.cancelParticipation();
uint256 balanceAfterCancel = asset.balanceOf(victim);
uint256 actualRefund = balanceAfterCancel - balanceBeforeCancel;
vm.stopPrank();
console.log("Balance after cancelParticipation():", balanceAfterCancel);
console.log("Actual refund received:", actualRefund);
console.log("Expected refund:", 594 ether);
console.log("Missing funds:", 594 ether - actualRefund);
// Phase 3: Verify the catastrophic fund loss
assertEq(vault.balanceOf(victim), 0, "All shares burned");
assertEq(vault.stakedAsset(victim), 0, "Staked asset cleared");
assertLt(actualRefund, 594 ether, "Victim received less than expected");
assertEq(actualRefund, 198 ether, "Victim only got last deposit refunded");
console.log("=== ACCOUNTING BUG CONFIRMED ===");
console.log("Victim deposited 600 ether total");
console.log("Victim expected 594 ether refund (minus 6 ether fees)");
console.log("Victim actually received 198 ether refund");
console.log("Victim permanently lost 396 ether!");
}
/**
* Demonstrate the root cause: deposit() overwrites stakedAsset
*/
function test_RootCause_StakedAssetOverwrite() public {
vm.startPrank(victim);
// Track what should accumulate vs what actually gets stored
console.log("=== ROOT CAUSE ANALYSIS ===");
// First deposit
vm.warp(EVENT_START - 1 hours);
vault.deposit(200 ether, victim);
uint256 expectedTotalStaked = 198 ether; // After 2 ether fee
uint256 actualTotalStaked = vault.stakedAsset(victim);
console.log("After first deposit:");
console.log("- Expected total staked:", expectedTotalStaked);
console.log("- Actual stakedAsset value:", actualTotalStaked);
// Second deposit
vault.deposit(200 ether, victim);
expectedTotalStaked += 198 ether; // Should accumulate
actualTotalStaked = vault.stakedAsset(victim);
console.log("After second deposit:");
console.log("- Expected total staked:", expectedTotalStaked);
console.log("- Actual stakedAsset value:", actualTotalStaked);
console.log("- PROBLEM: stakedAsset overwritten instead of accumulated!");
// Third deposit
vault.deposit(200 ether, victim);
expectedTotalStaked += 198 ether; // Should accumulate
actualTotalStaked = vault.stakedAsset(victim);
console.log("After third deposit:");
console.log("- Expected total staked:", expectedTotalStaked);
console.log("- Actual stakedAsset value:", actualTotalStaked);
console.log("- DISASTER: Lost track of 396 ether from prior deposits!");
vm.stopPrank();
// Verify the state corruption
assertEq(expectedTotalStaked, 594 ether, "Expected total should be 594 ether");
assertEq(actualTotalStaked, 198 ether, "But stakedAsset only tracks last deposit");
assertLt(actualTotalStaked, expectedTotalStaked, "State corrupted - lost track of user funds");
}
}
Recommended Mitigation
Modify deposit() to accumulate staked assets instead of overwriting:
- function deposit(uint256 assets, address receiver) public override returns (uint256) {
- // ... fee calculation ...
- uint256 stakeAsset = assets - fee;
- stakedAsset[receiver] = stakeAsset; // PROBLEM: OVERWRITES instead of accumulating!
- // ... rest of function ...
- }
+ function deposit(uint256 assets, address receiver) public override returns (uint256) {
+ // ... fee calculation ...
+ uint256 stakeAsset = assets - fee;
+ stakedAsset[receiver] += stakeAsset; // ACCUMULATE instead of overwrite
+ // ... rest of function ...
- }
Updates
Appeal created
bube 19 days ago
Submission Judgement Published 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.
Rewards breakdown
nSLOC
193This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.