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

Staked asset overwritten on repeated deposit

neomartis

Staked asset overwritten on repeated deposit

Description

  • Normal behavior: When a user deposits multiple times before the event starts, their total staked amount should be the sum of all net deposits (after fees), so accounting and later prize distribution reflect the full contribution. Or otherwise multiple deposits should be prohibited and guarded.

  • Issue: The contract overwrites the previous stake on every deposit by assigning stakedAsset[receiver] = stakeAsset instead of accumulating it. This under-reports the user’s total stake and leads to incorrect outcomes.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
...
uint256 stakeAsset = assets - fee;
​
// @> overwrites previous stake
stakedAsset[receiver] = stakeAsset;
...
}

Risk

Likelihood: High

  • Happens whenever a participant deposits more than once before event start.

  • Common behavior for users adjusting their stake; there is no restriction against multiple deposits.

Impact: High

  • Loss of tokens from user as on a win condition, not all stakes are counted

  • Results in leftover tokens in the vault and misaligned payouts, as later logic uses incorrect stake-related values, regardless if the user wins or not, there will be leftover tokens in the vault.

Proof of Concept

Description:

  • user1 deposits and joins, then deposits and joins again (same country).

  • stakedAsset[user1] reflects only the last deposit after fee, not the sum of both.

  • After end and withdrawal, vault shows leftover funds due to mis-accounting. These are locked forever and cannot be claimed anymore.

function testStakedAssetOverwrittenOnRepeatDeposits() public {
vm.prank(owner);
briVault.setCountry(countries);
​
// User 1 deposits and joins
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user1);
briVault.joinEvent(8);
vm.stopPrank();
​
// User 1 deposits again and joins again
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user1);
briVault.joinEvent(8);
vm.stopPrank();
​
// User 1 should have 2 ether staked
uint256 stakedAsset = briVault.stakedAsset(user1);
console.log("staked asset", stakedAsset);
// 985000000 000000000 (should be 2 ether minus the fee, but is only 1 ether)
​
// Ending the game and withdrawing
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(8);
vm.stopPrank();
​
vm.startPrank(user1);
briVault.withdraw();
vm.stopPrank();
​
console.log("balance user 1", mockToken.balanceOf(user1));
// 18 985000000 000000000
console.log("tokens left in the vault", mockToken.balanceOf(address(briVault)));
// 985000000 000000000
}

Recommended Mitigation

  • Accumulate stake across deposits to reflect the total net amount contributed by the receiver.

  • Consider preventing multiple joinEvent calls per address per event or recomputing recorded shares if business logic requires a fresh snapshot on each join.

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

Appeal created

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