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

Multiple Deposits Overwrite Previous Deposits Instead of Accumulating

jufel

Description:

The deposit() function assigns the deposit amount to stakedAsset[receiver] using the = operator instead of +=:

stakedAsset[receiver] = stakeAsset;

This means if a user deposits multiple times before the event starts, only their most recent deposit is tracked. Previous deposits are lost from the accounting system, even though:

  1. All deposits are transferred to the vault

  2. Shares are minted for each deposit

  3. The user paid participation fees for each deposit

Impact:

  • Users who deposit multiple times lose tracking of earlier deposits

  • If user deposits 1000, then 500, stakedAsset[user] shows only 500 (not 1500)

  • User has shares from both deposits but accounting is wrong

  • joinEvent() will only use shares from the last deposit in calculations

  • Users effectively lose previous deposit amounts from the betting pool consideration

  • Participation fees are charged multiple times but only last deposit counts

Proof of Concept:

function testMultipleDepositsOverwritePrevious() public {
uint256 firstDeposit = 10000 * 10**18;
uint256 secondDeposit = 5000 * 10**18;
​
// First deposit
vm.startPrank(attacker);
asset.approve(address(vault), firstDeposit + secondDeposit);
​
uint256 shares1 = vault.deposit(firstDeposit, attacker);
uint256 stakedAfterFirst = vault.stakedAsset(attacker);
​
// Second deposit
uint256 shares2 = vault.deposit(secondDeposit, attacker);
uint256 stakedAfterSecond = vault.stakedAsset(attacker);
​
vm.stopPrank();
​
// User has shares from both deposits
uint256 totalShares = vault.balanceOf(attacker);
assertEq(totalShares, shares1 + shares2, "User has combined shares");
​
// But stakedAsset only reflects second deposit!
assertEq(stakedAfterSecond, secondDeposit - (secondDeposit * 100 / 10000),
"Only second deposit tracked");
assertTrue(stakedAfterSecond < stakedAfterFirst + (secondDeposit - (secondDeposit * 100 / 10000)),
"First deposit was overwritten, not accumulated");
​
// User lost 10000 tokens from accounting
console.log("Expected staked:", stakedAfterFirst + (secondDeposit * 99 / 100));
console.log("Actual staked:", stakedAfterSecond);
}

Mitigation:

Use += to accumulate deposits:

function deposit(uint256 assets, address receiver) public override returns (uint256) {
// ... rest of the function
​
- stakedAsset[receiver] = stakeAsset;
+ stakedAsset[receiver] += stakeAsset; // Accumulate instead of overwrite
​
// ... rest of the function
}
Updates

Appeal created

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