BriVault

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

deposit() Function Mints Shares to Wrong Address — Ignoring receiver Parameter and Creating Ownership & Event Inconsistencies

Root + Impact

Description

  • Normal behavior:
    The deposit(uint256 assets, address receiver) should mint vault shares to the specified receiver address. The receiver is intended to be the beneficiary of the deposit, not necessarily the caller. The function is supposed to buy the assets for others, but it mints to msg.sender instead.

  • Issue:
    In the current implementation, the function accepts a receiver parameter but ignores it when minting shares, always minting to msg.sender instead. At the same time, it emits the deposited(receiver, value) event and records stakedAsset[msg.sender].
    This inconsistency leads to misaligned ownership records:

    • The event log and function parameters suggest that receiver received the deposit.

    • In reality, msg.sender owns the minted shares and recorded stake.
      This discrepancy can mislead front-ends, off-chain indexers, and users, creating false attribution of ownership and potential loss of expected returns for the intended receiver.

function deposit(uint256 assets, address receiver)
public override returns (uint256)
{
require(receiver != address(0));
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
uint256 fee = _getParticipationFee(assets);
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
uint256 stakeAsset = assets - fee;
@> stakedAsset[msg.sender] = stakeAsset; // tracked for sender only
@> uint256 participantShares = _convertToShares(stakeAsset);
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
@> _mint(msg.sender, participantShares); // shares minted to sender, not receiver
@> emit deposited(receiver, stakeAsset); // misleading: receiver shown in event
return participantShares;
}

Risk

Likelihood:

  • Occurs whenever receiver != msg.sender.

  • Common in scenarios such as staking on behalf of another user, integrations with DAOs, or deposit relayers.

Impact:

  • Accounting mismatch between event logs and on-chain state.

  • Receiver never actually receives shares, while they appear to have deposited assets.

  • Could break integrations, create user confusion, or lead to loss of rewards or withdrawals if the wrong account is recognized as depositor.

Proof of Concept

This PoC demonstrates the misleading behavior when receiver differs from msg.sender.

function testWrongReceiver() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
vm.stopPrank();
// Actual on-chain behavior
assertEq(briVault.stakedAsset(user1), 4.925 ether); // recorded under user1
assertEq(briVault.stakedAsset(user2), 0); // receiver not credited
// Event log misleadingly shows receiver as depositor
// Shares are owned by user1, not user2
}

This shows that while the event and parameter imply user2 deposited, the shares and staked amount belong to user1, creating inconsistent state.

Recommended Mitigation

Align the function logic with ERC-4626 standard by minting shares to the receiver and recording the correct ownership mapping.
This ensures the depositor and receiver relationship remains consistent both on-chain and in emitted events.

- stakedAsset[msg.sender] = stakeAsset;
- uint256 participantShares = _convertToShares(stakeAsset);
- IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
- IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
- _mint(msg.sender, participantShares);
- emit deposited(receiver, stakeAsset);
+ stakedAsset[receiver] = stakeAsset;
+ uint256 participantShares = _convertToShares(stakeAsset);
+ IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
+ IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
+ _mint(receiver, participantShares);
+ emit deposited(receiver, stakeAsset);
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!