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

Share Theft by Design: ERC4626 deposit mints to caller instead of receiver

0p71m4lpr1m3

Root + Impact

// src/briVault.sol (commit 1f515387d58149bf494dc4041b6214c2546b3b27)
// L207: function deposit(uint256 assets, address receiver) public override returns (uint256) { … }
// L231: _mint(msg.sender, participantShares); // BUG: ERC4626 says mint to `receiver`

File: src/briVault.sol • L207 (deposit header), L231 (mint) • Commit: 1f515387d58149bf494dc4041b6214c2546b3b27

Description

  • Expected (ERC4626): deposit(assets, receiver) must mint vault shares to receiver.

Actual (current code): stake is credited to receiver, but shares are minted to msg.sender. This breaks ERC4626 semantics and the vault’s join/withdraw flow. A user can deposit “for” another account; the receiver can join (stake recorded) but has 0 shares, so they receive 0 payout even if they picked the winner. The depositor holds the shares but cannot join (no stake), making redemption impossible via the custom withdraw() and silently redistributing value.

// src/briVault.sol (root cause)
function deposit(uint256 assets, address receiver) public override returns (uint256)
...
stakedAsset[receiver] = stakeAsset;
uint256 participantShares = _convertToShares(stakeAsset);
...
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
_mint(msg.sender, participantShares); // @ BUG: should mint to `receiver`

Risk

Likelihood:

  • Occurs whenever receiver ≠ msg.sender (common in zaps/routers/wrappers and “deposit for” user flows).


Impact:

  • Ownership/stake mismatch → receiver gets 0 payout; depositor can’t withdraw; funds misassigned and redistributed; breaks ERC4626 integrations and tooling.

Proof of Concept(PoC)

Threat model(what can go wrong):
Any flow where receiver != msg.sender (very common in zaps, routers, referral programs, or custodial UX) will misassign ownership of shares. Stake/accounting is recorded for receiver, but shares are minted to the caller, so:

  • receiver can join the tournament/event (stake recorded) but gets 0 payout (has 0 shares).

  • msg.sender (the caller) silently receives the shares and can later redeem/benefit.

  • This breaks ERC-4626 semantics and downstream integrations that rely on Deposit(caller, receiver, assets, shares) + balanceOf(receiver) parity.

High-severity ERC-4626 spec violation (direct deposit)

Preconditions: depositor owns 10e18 asset units and is not the receiver.
Expected (ERC-4626): shares mint to receiver.
Actual (current code): shares mint to caller (msg.sender).

// test/ZZ_Findings.t.sol (excerpt)
function test_Deposit_MintsToReceiver_NotMsgSender() public {
address depositor = address(0xD3AD);
address receiver = address(0xBEEF);
deal(address(asset), depositor, 10e18);
​
// Pre-state: both have 0 shares
assertEq(vault.balanceOf(depositor), 0);
assertEq(vault.balanceOf(receiver), 0);
​
vm.startPrank(depositor);
IERC20(address(asset)).approve(address(vault), 10e18);
​
// Deposit "for" receiver (receiver != msg.sender)
uint256 sh = vault.deposit(10e18, receiver);
vm.stopPrank();
​
// 🔴 Fails on buggy code: shares are minted to msg.sender, not receiver
assertEq(vault.balanceOf(receiver), sh, "ERC4626: shares must mint to receiver");
assertEq(vault.balanceOf(depositor), 0, "depositor should not hold the minted shares")

How to run the PoC

Single test:

forge test -vv --mc ZZ_Findings --mt test_Deposit_MintsToReceiver_NotMsgSender

All variants with traces:

forge test -vvvv --mc ZZ_Findings \
--mt test_Deposit_MintsToReceiver_NotMsgSender \
--mt test_Zap_RoutesAssetsButKeepsShares \
--mt test_Event_LooksCorrectButOwnershipWrong

Pass/Fail meaning

  • Fail (current code): assertions expecting shares at receiver fail because the contract mints to msg.sender.

Pass (after patch): same tests pass when the mint target is changed to receiver.


Recommended Mitigation

Why this is works :

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

  • Invariant: share owner = stake owner → balanceOf(receiver)+=shares; caller unchanged; Deposit(caller, receiver, assets, shares) matches state.

  • Safety: no storage/ACL/ABI changes; negligible gas; 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

Mitigation :
Patch: _mint(msg.sender, shares) → _mint(receiver, shares).

3) How to run

forge test -vv --mc ZZ_Findings_AfterFix --mt test_Deposit_MintsToReceiver_AfterFix

Updates

Appeal created

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