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

Overwriting Staked Amounts on Multiple Deposits

cryptostellar5

Root + Impact

Description

stakedAsset[receiver] = stakeAsset; overwrites the value instead of accumulating it. If a user deposits multiple times, only the last deposit is tracked, but shares are minted additively. This causes incorrect refunds in cancelParticipation.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
​
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
uint256 fee = _getParticipationFee(assets);
// charge on a percentage basis points
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
​
uint256 stakeAsset = assets - fee;
​
@> stakedAsset[receiver] = stakeAsset;
​
uint256 participantShares = _convertToShares(stakeAsset);
​
​
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
​
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
​
_mint(msg.sender, participantShares);
​
​
emit deposited (receiver, stakeAsset);
​
return participantShares;
}

Risk

Likelihood:

  • Users deposit multiple times before the event starts to increase their bet size or correct earlier amounts, triggering the overwrite of stakedAsset on every subsequent deposit.

Impact:

  • Users who cancel participation after multiple deposits lose all but the last deposited stake, with the difference (e.g., 98.5 ETH in the POC) becoming permanently stuck in the vault and donated to winners.

  • The vault accumulates unclaimed funds with zero shares outstanding, creating accounting inconsistencies and enabling unfair windfalls for winning participants.

Proof of Concept

Assume fee = 0 for simplicity, minimumAmount = 0.

  • User A deposits 100: stakedAsset[UserA] = 100, shares minted = 100.

  • User A deposits 200: stakedAsset[UserA] = 200 (overwrites), additional shares minted = 200 (total shares = 300). Vault balance = 300.

  • User A calls cancelParticipation: refundAmount = 200, burns 300 shares, transfers 200 back.

  • Result: Vault has 300 - 200 = 100 left, but no shares outstanding. The extra 100 is stuck/donated to the pool.

function test_StakedAmountOverwriting() public {
vm.prank(owner);
briVault.setCountry(countries);
​
uint256 deposit1 = 100 ether;
uint256 deposit2 = 200 ether;
mockToken.mint(user1, deposit1 + deposit2); // 300 ETH
​
// === DEPOSIT 1 ===
vm.startPrank(user1);
mockToken.approve(address(briVault), type(uint256).max);
uint256 shares1 = briVault.deposit(deposit1, user1);
vm.stopPrank();
​
console.log("After deposit 1:");
console.log(" stakedAsset[user1]:", briVault.stakedAsset(user1));
console.log(" shares[user1]:", briVault.balanceOf(user1));
console.log(" vault balance:", mockToken.balanceOf(address(briVault)));
​
// === DEPOSIT 2 (OVERWRITES stakedAsset) ===
vm.prank(user1);
uint256 shares2 = briVault.deposit(deposit2, user1);
​
console.log("After deposit 2:");
console.log(" stakedAsset[user1]:", briVault.stakedAsset(user1));
console.log(" total shares:", briVault.balanceOf(user1));
console.log(" vault balance:", mockToken.balanceOf(address(briVault)));
​
// === CANCEL PARTICIPATION (REFUNDS ONLY LAST DEPOSIT) ===
uint256 vaultBefore = mockToken.balanceOf(address(briVault));
vm.prank(user1);
briVault.cancelParticipation();
​
console.log("After cancelParticipation:");
console.log(" stakedAsset[user1]:", briVault.stakedAsset(user1)); // 0
console.log(" shares[user1]:", briVault.balanceOf(user1)); // 0
console.log(" vault balance:", mockToken.balanceOf(address(briVault))); // assets stuck !
​
uint256 stuck = mockToken.balanceOf(address(briVault));
assertEq(stuck, 98.5 ether, "98.5 ETH stuck: first deposit lost due to overwrite");
}

Recommended Mitigation

Change stakedAsset[receiver] = stakeAsset; to stakedAsset[receiver] += stakeAsset; to correctly track total user stake across multiple deposits. This ensures cancelParticipation() refunds the full deposited amount, preventing permanent fund loss.

- 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!