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

Misaccounting in stakedAsset Overwrites Leads to Vault Imbalance and Potential DoS

chobby

Description

  • The vault’s deposit() function overwrites existing entries in stakedAsset[user] instead of accumulating them.

    When a user deposits multiple times without canceling participation, their previous deposit record is erased in storage even though the tokens remain in the contract.

    As a result, the vault’s internal accounting becomes desynchronized — the vault’s actual token balance is greater than the total represented shares. This breaks withdrawal logic and can lead to an event’s payout failing or miscalculating.

    This issue shares the same root cause as the previously reported finding “Potential Share Dilution via stakedAsset[receiver] = stakeAsset”, but the impact here occurs after the event is completed, not when canceling participation.

<@ stakedAsset[receiver] = stakeAsset;

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

  • Reason 2

Impact:

The vault retains unaccounted tokens, causing inflated balances.

  • Winners receive incorrect withdrawal amounts.


  • Losers may be unable to withdraw their full refunds.


  • Residual unclaimed funds remain locked, potentially causing a Denial of Service (DoS) for future rounds or event settlements.

Proof of Concept

Steps to Reproduce:

  1. Alice deposits 5 ether and joins Team 1.

  2. Alice deposits another 5 ether and rejoins Team 1 (total 10 ether).

  3. Two other users join and deposit 5 ether each.

  4. The event concludes and Team 1 is set as the winner.

  5. When users withdraw, the vault retains leftover balance due to overwritten accounting for Alice’s first deposit.

Observed output logs:

Vault Balance before withdrawals is 19700000000000000000
Vault Balance after withdrawals is 6566666666666666668
alice Balance after withdrawals is 16566666666666666666
user1 Balance after withdrawals is 18283333333333333333
user2 Balance after withdrawals is 18283333333333333333


Notice the vault still holds ~6.56 ETH even after all winners withdrew — proving funds were left behind because Alice’s first deposit was overwritten and never accounted for.

​
function testAccountingFlawWithStakedAssetsCouldLeadToDoS() public {
// we check vault balance before any deposits
console.log(
"Vault Balance Before deposits is",
mockToken.balanceOf(address(briVault))
);
​
// we check alice balance before ever joining team
console.log(
"alice Balance before joining team and making deposit is",
mockToken.balanceOf(alice)
);
​
// alice deposits and joins team 1
vm.prank(alice);
mockToken.approve(address(briVault), 5 ether);
vm.prank(alice);
briVault.deposit(5 ether, alice);
​
vm.prank(alice);
briVault.joinEvent(0);
​
// alice deposits again and joins team 1 making it total of 10 eth
vm.prank(alice);
mockToken.approve(address(briVault), 5 ether);
vm.prank(alice);
briVault.deposit(5 ether, alice);
​
vm.prank(alice);
briVault.joinEvent(0);
​
// we check user1 balance before ever joining team
console.log(
"user1 Balance before joining team and making deposit is",
mockToken.balanceOf(user1)
);
// user1 deposits and joins team 1
vm.prank(user1);
mockToken.approve(address(briVault), 5 ether);
vm.prank(user1);
briVault.deposit(5 ether, user1);
​
vm.prank(user1);
briVault.joinEvent(0);
​
// we check user2 balance before ever joining team
console.log(
"user2 Balance before joining team and making deposit is",
mockToken.balanceOf(user2)
);
​
// user2 deposits and joins team 1
vm.prank(user2);
mockToken.approve(address(briVault), 5 ether);
vm.prank(user2);
briVault.deposit(5 ether, user2);
​
vm.prank(user2);
briVault.joinEvent(0);
​
// Fast forward to after event end
vm.warp(block.timestamp + 60 days);
vm.roll(block.number + 1);
// Owner chooses team 1 at 0 index
vm.prank(owner);
briVault.setWinner(0);
​
console.log(
"Vault Balance before withdrawals is",
mockToken.balanceOf(address(briVault))
);
​
vm.prank(alice);
briVault.withdraw();
​
vm.prank(user1);
briVault.withdraw();
​
vm.prank(user2);
briVault.withdraw();
​
console.log(
"Vault Balance after withdrawals is",
mockToken.balanceOf(address(briVault))
);
console.log(
"alice Balance after withdrawals is",
mockToken.balanceOf(alice)
);
console.log(
"user1 Balance after withdrawals is",
mockToken.balanceOf(user1)
);
console.log(
"user2 Balance after withdrawals is",
mockToken.balanceOf(user2)
);
}
​
​
​
​
​

Recommended Mitigation

Accumulate deposits instead of overwriting user state:

This issue shares the same root cause as the “Potential Share Dilution” finding but affects different execution paths.

While the first finding impacts refunds after cancellation, this one affects post-event withdrawals, making it a separate, high-severity systemic flaw.

\

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

Appeal created

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