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

Potential Share Dilution via stakedAsset[receiver] = stakeAsset;

chobby

Overwriting Depositor’s Stake Allows Share Dilution or Loss of Accounting Accuract

Description

  • In the deposit() function:

    stakedAsset[receiver] = stakeAsset; sets stakeAsset to the user

  • But unfortunately when a user deposits more then once before the event starts, the previous deposit gets overwritten.

stakedAsset[receiver] = stakeAsset;

Risk

Likelihood:

  • it will happen every single time any user makes more than one deposit

  • if multiple users make multiple deposits, they all loose their initial funds. only the last one gets accounted for.

Impact:

  • Users could lose refund value if they cancel participation (refund uses stakedAsset).


  • Accounting errors break _getWinnerShares() and withdraw() fairness.


  • Allows malicious users to manipulate share-to-asset ratio before event start

Proof of Concept

1. alice deposits 5 tokens — stakedAsset[user] = 5.

2. alice deposits again with 5 tokens — now stakedAsset[user] = 10, but total shares = 300 worth.

3.	If user calls cancelParticipation(), they only get 5 refunded instead of 10.
function testShareDilutionofAssetsWhenDepositMoreThanOnce() public {
// 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 alice balance before cancelling
console.log(
"alice Balance before cancelling deposit is",
mockToken.balanceOf(alice)
);
​
//alice cancels deposit
vm.prank(alice);
briVault.cancelParticipation();
​
// we check alice balance after cancelling
console.log(
"alice Balance after cancelling deposit is",
mockToken.balanceOf(alice)
);
}

Recommended Mitigation

Accumulate instead of overwrite.

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