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

Incorrect share minting in deposit (mints to msg.sender instead of receiver)

osamaebaid

Root + Impact

Description

  • The deposit function is intended to allow users to deposit assets and receive vault shares in return, with the shares being credited to the specified receiver address.

  • However, the function incorrectly mints the shares to msg.sender instead of the intended receiver address, breaking the expected ERC-4626 behavior where shares should be assigned to the 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[receiver] = stakeAsset;
​
uint256 participantShares = _convertToShares(stakeAsset);
​
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
​
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
// INCORRECT: Mints to msg.sender instead of receiver
@>_mint(msg.sender, participantShares);
​
emit deposited (receiver, stakeAsset);
​
return participantShares;
}

Risk

Likelihood: Medium

  • This issue will occur every time a user deposits assets on behalf of another address (receiver ≠ msg.sender)

  • The problem will manifest in any integration that uses the deposit function with a receiver different from the caller

Impact: High

  • Shares are assigned to the wrong address, making the receiver unable to access their deposited funds

  • The depositor (msg.sender) receives shares they didn't pay for, while the intended receiver gets nothing

  • Breaks ERC-4626 compliance and makes the vault incompatible with DeFi protocols expecting standard behavior

Proof of Concept

  • Add testDepositSharesMintedToWrongAddress to briVault.t.sol

  • To verify the cause is that reciever balance of shares zero,run forge test --mt testDepositSharesMintedToWrongAddress -vvvv

function testDepositSharesMintedToWrongAddress() public {
// User deposits
vm.prank(user1);
mockToken.approve(address(briVault), 1 ether);
// user1 deposits 1 ether tokens on behalf of user2
vm.prank(user1);
briVault.deposit(1 ether, user2);
//user2 has no shares
assertEq(briVault.balanceOf(user2), 0);
// user2 join event
vm.prank(user2);
briVault.joinEvent(0);
// Advance time past event end
vm.warp(eventEndDate + 1);
// owner set winner
vm.prank(owner);
briVault.setWinner(0);
// user2 cannot withdraw as he has no shares
vm.prank(user2);
vm.expectRevert();
briVault.withdraw();
}

Recommended Mitigation

  • Change the minting target from msg.sender to 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);
// charge on a percentage basis points
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
​
uint256 stakeAsset = assets - fee;
​
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);
+ _mint(receiver, participantShares);
​
emit deposited (receiver, stakeAsset);
​
return participantShares;
}
Updates

Appeal created

bube Lead Judge 21 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!