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

ERC4626 Deposit Mints Shares to Caller, Trapping Receiver Funds

galer ah

Root + Impact

Description

deposit(uint256 assets, address receiver) should mint vault shares to receiver, but BriVault calls _mint(msg.sender, participantShares) (src/briVault.sol:231). Integrators that deposit on behalf of users transfer the assets, yet the caller retains the shares. The receiver’s stakedAsset is updated, allowing them to join, but because balanceOf(receiver) == 0, they can never withdraw. The caller holds shares but is not mapped to any team and fails the winner check.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
uint256 stakeAsset = assets - fee;
@> stakedAsset[receiver] += stakeAsset;
...
@> _mint(msg.sender, participantShares);
}

Risk

Likelihood: High – delegated deposits are standard for ERC4626 vaults (e.g., routers, smart accounts).

Impact:

  • Receivers lose 100% of the deposit; callers hold unusable shares.

  • Breaks ERC4626 compatibility, blocking integrations.

Proof of Concept

  1. Account A calls deposit(5 ether, accountB).

  2. stakedAsset[accountB] shows a balance, but balanceOf(accountB) == 0.

  3. Account B cannot withdraw winnings because they never receive shares; Account A cannot withdraw because they have no team registration.
    (Reproduced in test/BriVaultAttack.t.sol:219 via testDepositForDifferentReceiverMintsSharesToCaller.)

function testDepositForDifferentReceiverMintsSharesToCaller() public {
vm.prank(winner);
briVault.deposit(FIRST_DEPOSIT, victimOne);
​
assertEq(briVault.balanceOf(victimOne), 0); // receiver got no shares
assertEq(briVault.balanceOf(winner), _netStake(FIRST_DEPOSIT)); // caller holds them
}

Recommended Mitigation

  1. Mint shares to receiver (_mint(receiver, participantShares)) and ensure all accounting follows the share holder.

  2. Update stakedAsset, userToCountry, and userSharesToCountry to align with whichever address actually owns the shares.

  3. Add ERC4626 compliance tests covering delegated deposits.

Proposed patch (Solidity-like pseudocode):

function deposit(uint256 assets, address receiver) public override returns (uint256) {
- stakedAsset[receiver] += stakeAsset;
- _mint(msg.sender, participantShares);
+ stakedAsset[receiver] += stakeAsset;
+ _mint(receiver, participantShares);
+ _afterShareChange(receiver);
}
​
function joinEvent(uint256 countryId) public {
- if (stakedAsset[msg.sender] == 0) revert noDeposit();
+ if (balanceOf(msg.sender) == 0) revert noDeposit();
Updates

Appeal created

bryanconquer Lead Judge 16 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!