Root + Impact

Description

• Expected (ERC-4626): deposit(assets, receiver) mints shares to receiver.

Actual (current code): mints shares to msg.sender while stake is attributed to receiver.

Risk

Likelihood:

• Triggers whenever receiver ≠ msg.sender (common in zaps/wrappers/custodial UX).
  Impact: Integration/UX breakage and misattribution—not direct asset theft → Medium.

Impact:

• (Medium): When receiver != msg.sender, shares mint to the caller while stake/accounting is recorded for receiver. This breaks ERC-4626 semantics and downstream integrations (UIs/zaps/indexers expecting balanceOf(receiver) and Deposit(caller, receiver, assets, shares) parity), causing blocked withdrawals/joins and accounting drift—but no immediate loss of principal.

Proof of Concept(PoC)

The vault implements ERC-4626. Per the spec, deposit(assets, receiver) must mint vault shares to receiver (the beneficiary of the deposit), not to the caller. This preserves the invariant “stake owner == share owner.”

Vulnerability
In deposit, stake/accounting is updated for receiver, but the implementation calls
_mint(msg.sender, participantShares).
Thus, state says receiver staked, while ownership says the caller owns the shares.

Exploitation surface
Any flow where receiver != msg.sender—zaps, routers, referrals, custodial UX, batch joins—causes silent misassignment. No privileges are required.

Expected vs. Actual

• Expected (ERC-4626): sh = convertToShares(assets) and _mint(receiver, sh).

• Actual (bug): sh is minted to msg.sender; receiver gets 0 shares.

Security & functional impact

• Ownership/Stake split: receiver can “join” (stake recorded) yet has 0 shares → 0 payout.

• Withdraw/Redemption broken: Depositor holds shares but has no stake, so the redeem path is inconsistent or blocked by stake checks.

• Downstream breakage: Front-ends, routers, indexers relying on Deposit(caller, receiver, assets, shares) parity and on balanceOf(receiver) are wrong.

• Value leakage: Caller can later redeem the mis-minted shares.

Repro (what the test does)

1. Setup: Mint 10e18 asset units to depositor and approve the vault.

2. Action: vault.deposit(10e18, receiver) with receiver != depositor.

3. Assertions (current code fails):

   • assertGt(sh, 0); // sanity on conversion

   • assertEq(vault.balanceOf(receiver), sh); // fails (receiver got 0)

   • assertEq(vault.balanceOf(depositor), 0); // fails (depositor wrongly owns sh)

Run

How to run the PoC

Pass/Fail meaning

• Fail (current code): assertion balanceOf(receiver) == sh fails → shares minted to caller.

• Pass (after patch): same test passes → shares minted to receiver.

Recommended Mitigation

**Why this fix works **

• ERC-4626 rule: deposit(assets, receiver) → shares to receiver; caller-mint violates spec.

Invariant restore: balanceOf(receiver) += shares; events/semantics match; caller unchanged.

• Safety: No storage/ACL/ABI changes; negligible gas delta; restores ERC-4626 integrator compatibility.

  diff --git a/src/briVault.sol b/src/briVault.sol

  @@

  - _mint(msg.sender, participantShares); // BUG: mints to caller

  + _mint(receiver, participantShares); // FIX: mint to `receiver` per ERC-4626
  
  

  After fix

  function test_Deposit_MintsToReceiver_AfterFix() public

  address depositor = address(0xD3AD);

  address receiver = address(0xBEEF);

  deal(address(asset), depositor, 10e18);

  vm.startPrank(depositor);

  IERC20(address(asset)).approve(address(vault), 10e18);

  uint256 sh = vault.deposit(10e18, receiver);

  vm.stopPrank();

  ​

  assertEq(vault.balanceOf(receiver), sh);

  assertEq(vault.balanceOf(depositor), 0);
  forge test -vv --mc ZZ_Findings --mt test_Deposit_MintsToReceiver_AfterFix