BriVault

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

Incorrect Minting of Shares to `msg.sender` Instead of `receiver` in `BriVault::deposit` function (ERC4626 Violation + Asset Loss Risk)

Root + Impact

Incorrect Minting of Shares to msg.sender Instead of receiver in BriVault::deposit function (ERC4626 Violation + Asset Loss Risk)

Description

Normal behavior

Under ERC-4626 specification, when a user calls deposit(assets, receiver), the vault must mint shares directly to the receiver. The caller (msg.sender) provides the tokens, but the receiver is the beneficiary and must receive the vault shares representing ownership of the deposited assets.

Specific issue

In the current implementation, the briVault::deposit function incorrectly mints shares to msg.sender instead of receiver. As a result, whenever a smart contract deposits on behalf of a user such as a proxy contract the shares are credited to the proxy rather than the intended user. This breaks ERC-4626 compliance and causes misallocation of ownership, potentially locking users out of their assets and enabling privilege abuse or fee capture by unintended parties.

// Root cause in the codebase with @> marks to highlight the relevant section
function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
uint256 fee = _getParticipationFee(assets);
// charge on a percentage basis points
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
uint256 stakeAsset = assets - fee;
// record staked amount
stakedAsset[receiver] = 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);
return participantShares;
}

Risk

Likelihood:

  • Any deposit performed through a smart contract, relay, or aggregator automatically triggers the misallocation because msg.sender receives the shares instead of the receiver.

  • Automated vault interactions, multi-call transactions, and account abstraction systems regularly use proxy contracts, making this behavior likely in standard usage scenarios.

Impact:

  • Deposited assets do not result in ownership of shares for the intended receiver, preventing withdrawals or participation in rewards and governance.

  • Malicious or misconfigured intermediaries can capture shares, causing loss of funds or stuck assets for end users.

Proof of Concept

This PoC reproduces the issue by deploying a contract DepositProxy that calls deposit(...) on behalf of a receiver. The test mints ERC20 tokens to the proxy, makes the proxy approve the vault, performs the deposit, and asserts that the proxy not the receiver received the minted shares.

  • The proxy (msg.sender) incorrectly receives all minted shares

  • The receiver receives none, despite depositing assets

  • Confirms real-world exploitability via proxy/layered calls

contract DepositProxy {
BriVault public vault;
constructor(BriVault _briVault) {
vault = _briVault;
}
function depositFor(uint256 amount, address receiver) public {
vault.deposit(amount, receiver);
}
}
function test_deposit_vulnerability_minting_to_proxy() public {
DepositProxy proxy = new DepositProxy(briVault);
//give proxy some tokens
mockToken.mint(address(proxy), 10 ether);
//impersonate proxy
vm.startPrank(address(proxy));
mockToken.approve(address(briVault), 5 ether);
//balance before deposit
uint256 proxyInitialShares = briVault.balanceOf(address(proxy));
uint256 receiverInitialShares = briVault.balanceOf(user2);
console.log("Proxy initial shares:", proxyInitialShares);
console.log("Receiver initial shares:", receiverInitialShares);
//call depositFor
proxy.depositFor(5 ether, user2);
vm.stopPrank();
//balances after
uint256 proxyFinalShares = briVault.balanceOf(address(proxy));
uint256 receiverFinalShares = briVault.balanceOf(user2);
console.log("Proxy final shares:", proxyFinalShares);
console.log("Receiver final shares:", receiverFinalShares);
//vulnerability check, receiver has no shares
assertEq(receiverFinalShares, receiverInitialShares, "Receiver should get shares, but did not");
// proxy incorrectly receives shares
assertGt(
proxyFinalShares, proxyInitialShares, "proxy should not receive shares, but it did due to vulnerability"
);
}

Recommended Mitigation

  • Update the minting logic so shares are minted to the correct address

- _mint(msg.sender, participantShares);
+ _mint(receiver, participantShares);
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!