BriVault

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

Deposit inconsistency in `BriVault::deposit` allows depositor to receive shares for self while staking for another

Root + Impact

Description

  • The `BriVault::deposit(uint256 assets, address receiver)` function records the staked amount for `receiver` but mints ERC20 shares to `msg.sender`. This breaks accounting invariants: the vault believes `receiver` staked, but the depositor receives the shares.

// BriVault::deposit
stakedAsset[receiver] = stakeAsset;
_mint(msg.sender, participantShares);

Risk

Likelihood:

  • When third‑party deposits on behalf of other users are accepted by frontends or integrations (common UX that supports "deposit on behalf"), a malicious depositor can intentionally supply a different receiver value to mint shares to themselves while recording the stake under the victim address.

  • When off‑chain scripts or relayers submit transactions that pass a receiver differing from the payer (for example, gas‑sponsored deposits or meta‑transactions), the mismatch between stored stake and minted shares will occur and remain unnoticed without explicit invariants/tests.

Impact:

  • Depositor can deposit on behalf of another address but get the shares for themselves.

  • This allows the depositor to unfairly participate in events, claim rewards, or withdraw funds, effectively misappropriating tokens.

  • Accounting inconsistencies can cascade to other functions relying on stakedAsset vs balanceOf.

Proof of Concept

1. User2 approves and deposits 1 ether (mock ERC20 units) on behalf of user1.
2. `stakedAsset` is credited to user1, but ERC20 shares are minted to user2.
3. Inspect `stakedAsset` and balances.

Add the following to `briVault.t.sol`

function test_POC_InconsistencyDeposit() public {
uint256 depositAmount = 1 ether;
// user2 approves and deposits on behalf of user1
vm.prank(user2);
mockToken.approve(address(briVault), depositAmount);
vm.prank(user2);
briVault.deposit(depositAmount, user1);
// stakedAsset stored for receiver (user1), but shares minted to msg.sender (user2)
uint256 stakedForUser1 = briVault.stakedAsset(user1);
uint256 stakedForUser2 = briVault.stakedAsset(user2);
uint256 sharesUser1 = briVault.balanceOf(user1);
uint256 sharesUser2 = briVault.balanceOf(user2);
// Assertions demonstrating inconsistency
assertTrue(stakedForUser1 > 0, "stakedAsset for receiver should be > 0");
assertEq(stakedForUser2, 0, "stakedAsset for depositor should be 0 (since receiver used)");
assertEq(sharesUser1, 0, "receiver should not have received ERC20 shares");
assertTrue(sharesUser2 > 0, "depositor unexpectedly received shares");
// Optional console log
console.log("stakedForUser1:", stakedForUser1);
console.log("sharesUser2:", sharesUser2);
}

Recommended Mitigation

Mint shares to the `receiver` instead of `msg.sender` to align staked assets and ERC20 balances:

- stakedAsset[receiver] = stakeAsset;
- _mint(msg.sender, participantShares);
+ stakedAsset[receiver] = stakeAsset;
+ _mint(receiver, participantShares);
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Shares Minted to msg.sender Instead of Specified Receiver

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!