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

`Brivault` overrides `deposit` instead of `_deposit` function, causing shares to be minted to `msg.sender` rather than the intended `receiver` and bypassing `ERC4626’s:: _convertToShares`, which leads to inaccurate share calculations.

mafa

Brivault overrides deposit instead of _deposit function, causing shares to be minted to msg.sender rather than the intended receiver and bypassing ERC4626’s:: _convertToShares, which leads to inaccurate share calculations.

Description

The Brivault contract inherits from ERC4626 and overrides the deposit function. In ERC4626, overriding the internal _deposit function automatically affects both the deposit and mint mechanisms. The _deposit function is supposed to mint shares to the specified receiver using _mint(receiver, shares). However, in the overridden deposit function, shares are instead minted to msg.sender via _mint(msg.sender, participantShares).

Additionally, by overriding deposit directly, the contract bypasses the standard ERC4626 ::_convertToShares logic, resulting in imprecise share calculations that may not correctly reflect the proportional ownership based on the vault’s assets.

function deposit(uint256 assets, address receiver) public override returns (uint256) {

Risk

The result is that the receiver does not receive the expected minted tokens; instead, the msg.sender receives the tokens.

Proof of Concept

User1 approves Brivault and deposits on behalf of User2.As a result, User2 did not receive the shares.

Place the following code in briVault.t.sol.

function testReceiverCanNotGetTheShared() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
vm.stopPrank();
​
console2.log("user balance 1:", briVault.balanceOf(user1));
console2.log("user balance 2:", briVault.balanceOf(user2));
assertEq(briVault.balanceOf(user2), 0 );
}
​

The Result:

user balance 1: 4925000000000000000
user balance 2: 0

Recommended Mitigation

The _deposit function should be overridden instead of deposit.

+ function deposit(uint256 assets, address receiver) public override returns (uint256) {
+ return super.deposit(assets,receiver);}
- function deposit(uint256 assets, address receiver) public override returns (uint256) {...}
+ function _deposit(address caller,
+ address receiver, uint256 assets, int256 shares)internal override {
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!