BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: medium
Invalid

Shares Computed Before Tokens Arrive: Wrong Share Math Ordering

wyd_derick

Root + Impact

Description

  • The contract calculates vault shares before the deposited tokens are actually received.

  • This causes incorrect math in _convertToShares, letting deposits mint too many or too few shares depending on fee mechanics or deflationary tokens.

function deposit(uint256 assets, address receiver)
public
override
returns (uint256)
{
require(receiver != address(0));
​
@> uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this));
@> uint256 participantShares = _convertToShares(assets); // ❌ uses pre-deposit balance
@> IERC20(asset()).safeTransferFrom(msg.sender, address(this), assets);
​
_mint(receiver, participantShares);
emit deposited(receiver, assets);
return participantShares;
}
​

Risk

Likelihood:

  • This happens every time a user deposits, because the code always calls _convertToShares() before the assets arrive.

  • The issue is automatic and silent users don’t need to manipulate timing or values. Any normal deposit creates wrong math when using deflationary tokens or fee deductions.

Impact:

  • Over-minting shares allows certain users to receive more vault ownership than they actually deposited.

  • Breaks ERC4626 accounting, potentially causing long-term dilution or imbalance.

Proof of Concept

Explanation:

Because _convertToShares() used the vault’s old balance (before safeTransferFrom), the computed ratio was wrong.
When the tokens finally arrived, total assets increased but the minted shares didn’t match the real value.

function test_WrongShareMathOrdering() public {
address alice = address(0xA1);
asset.mint(alice, 100e18);
asset.approve(address(vault), 100e18);
​
vm.startPrank(alice);
vault.deposit(100e18, alice);
vm.stopPrank();
​
// If share math was correct, shares ≈ assets (1:1)
// Wrong ordering causes mismatch depending on vault balance before deposit
uint256 expectedShares = 100e18;
uint256 mintedShares = vault.balanceOf(alice);
assertFalse(mintedShares == expectedShares); // indicates bug
}
​

Recommended Mitigation

Explanation:

This ensures share calculations use the true number of tokens actually received, matching ERC4626 behavior.
It prevents inflation, under-minting, and maintains vault consistency across deposits.

function deposit(uint256 assets, address receiver)
public
override
returns (uint256)
{
require(receiver != address(0));
​
+ uint256 prevBalance = IERC20(asset()).balanceOf(address(this));
+ IERC20(asset()).safeTransferFrom(msg.sender, address(this), assets);
+ uint256 postBalance = IERC20(asset()).balanceOf(address(this));
+ uint256 actualReceived = postBalance - prevBalance;
​
+ uint256 totalShares = totalSupply();
+ uint256 shares;
+ if (totalShares == 0) {
+ shares = actualReceived; // 1:1 first deposit
+ } else {
+ shares = Math.mulDiv(actualReceived, totalShares, prevBalance);
+ }
​
- uint256 participantShares = _convertToShares(assets);
- _mint(receiver, participantShares);
+ _mint(receiver, shares); // ✅ correct share math
​
emit deposited(receiver, actualReceived);
return shares;
}
​
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!