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

Incorrect minting of ERC4626 shares to msg.sender instead of receiver causes broken ERC4626 compliance and incorrect accounting

aliismakh

Incorrect minting of ERC4626 shares to msg.sender instead of receiver causes broken ERC4626 compliance and incorrect accounting

Description

  • After calling BriVault::deposit() function, the vaults mints specified amount of shares to receiver.

  • But, it happens that the BriVault::deposit() function, the vault mints shares to msg.sender instead of the intended receiver.
    This breaks the expected ERC4626 behavior where the shares must be minted to the specified receiver.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
.
.
.
uint256 participantShares = _convertToShares(stakeAsset);
​
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
​
@> _mint(msg.sender, participantShares); // ❌ Should mint to `receiver`
emit deposited (receiver, stakeAsset);
​
return participantShares;
}

Risk

Likelihood:

  • This issue occurs whenever a user calls deposit() with a receiver different from msg.sender.

Impact:

  • The receiver will not receive any vault shares.

  • The depositor will receive shares that belong to someone else.

  • Withdrawals and share accounting will break, leading to fund misallocation and potentially locked or inaccessible funds.

  • ERC4626 compliance is broken, which may cause integration failures with DeFi tools that rely on standard behavior.

Proof of Concept

Add the following to briVault.t.sol and run forge test --mt testSharesNotMintToReceiver

function testSharesNotMintToReceiver() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2); // Deposit on behalf of user2
vm.stopPrank();
​
uint256 user2shares = briVault.balanceOf(user2);
assertEq(user2shares, 0); // ❌ Fails — user2 should have received shares
}

  • Expected behavior: user2 should receive shares equal to _convertToShares(assets - fee).

  • Actual behavior: Shares are minted to user1 instead.

  • Result:

Ran 1 test for test/briVault.t.sol:BriVaultTest
[PASS] testSharesNotMintToReceiver() (gas: 170319)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 48.97ms (7.72ms CPU time)
​
Ran 1 test suite in 471.71ms (48.97ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Replace the minting recipient in deposit() from msg.sender to receiver:

- _mint(msg.sender, participantShares);
+ _mint(receiver, participantShares);

This aligns the implementation with ERC4626 specifications and ensures that deposits made on behalf of another user credit the correct address.

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!