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

BriVault.deposit and BriVault.previewDeposit return different values, must return equal values

alexscherbatyuk

Root + Impact

Description

  • In the BriVault contract, which extends ERC4626, the deposit function is overridden to deduct a participation fee before minting shares based on the net staked assets, ensuring accurate tracking for tournament participation while complying with ERC4626 by returning the shares minted inclusive of fee effects.

  • However, previewDeposit is not overridden, relying on the base ERC4626 implementation that previews shares without accounting for the custom fee deduction, resulting in previewDeposit returning a higher share amount than what deposit actually mints, violating ERC4626's requirement for consistent preview and execution values.

// Root cause in the codebase with @> marks to highlight the relevant section
@> function previewDeposit(uint256 assets) public view virtual returns (uint256 shares) {
@> return _convertToShares(assets, Math.Rounding.Floor);
@> }

Risk

Likelihood:

  • Frontends and wallets call previewDeposit to display estimated shares before confirming deposits.

  • Actual deposits execute with fee deduction, occurring whenever users interact via standard ERC4626 interfaces.

Impact:

  • Users receive fewer shares than previewed, leading to confusion, perceived losses, and integration failures in DeFi composability.

  • Undermines ERC4626 compliance, exposing the protocol to exploits from tools assuming accurate previews and eroding integrator trust.

Proof of Concept

Add the following code snippet to the briVault.t.sol test file.This test verifies that users get different results by calling BriVault::deposit and BriVault::previewDeposit.

function test_previewDepositMustReturnSameValueAsDeposit() public {
vm.startPrank(user1);
uint256 AMOUNT = 5 ether;
mockToken.approve(address(briVault), AMOUNT);
uint256 depositShares = briVault.deposit(AMOUNT, user1);
uint256 previewDepositShares = briVault.previewDeposit(AMOUNT);
vm.stopPrank();
assertEq(depositShares, previewDepositShares);
}

Recommended Mitigation

Potential mitigation is to override the BriVault.::previewDeposit and add protocol fee calculation.

+ function previewDeposit(uint256 assets) public view override returns (uint256) {
+
+ uint256 fee = _getParticipationFee(assets);
+
+ if (minimumAmount + fee > assets) {
+ revert lowFeeAndAmount();
+ }
+
+ uint256 stakeAsset = assets - fee;
+
+ return _convertToShares(stakeAsset, Math.Rounding.Floor);
+ }
Updates

Appeal created

bube Lead Judge 20 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Unrestricted ERC4626 functions

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!