# The calculation of takeFees in Vault contract is incorrect

Vault.takeFees mints fee shares using pre-mint totalAssets, diluting the fee recipient so management fees are undercharged.

- Payout: Contest reward
- Severity: medium
- Program: Popcorn
- Credited: KingNFT
- Disclosed: 2023-09-07
- Write-up quality: 6/10
- Original: https://github.com/code-423n4/2023-01-popcorn-findings/issues/491
- Library page: https://bugbountylibrary.com/r/c4%3A2023-01-popcorn%3AM-16
- Tags: domain:web3 impact:funds-loss kind:contest platform:code4rena sev:medium tech:evm tech:solidity technique:source-review vuln:accounting-error vuln:fee-bypass

**Target:** Vault.sol takeFees modifier / ERC4626-style vault
**Root cause:** takeFees computes feeInShare via convertToShares(totalFee) using totalAssets() before the fee shares are minted, so the new shares are priced against pre-fee assets and the fee recipient receives fewer assets than the fee owed.
**Steps:**
1. Set fees.management = 0.5e18, deposit/withdrawal/performance = 0; vault holds totalAssets=100, totalSupply=100.
2. Advance block.timestamp by SECONDS_PER_YEAR; accruedManagementFee() returns 50 $AST.
3. Trigger any function with takeFees modifier; totalFee=50, currentAssets=100.
4. _mint(feeRecipient, convertToShares(50)) = 50*100/100 = 50 $SHR; totalSupply becomes 150.
5. convertToAssets(50 $SHR) = 50*100/150 = 33.3 $AST, less than the 50 $AST owed.
**How found:** Code reading of the takeFees modifier: noticed convertToShares(totalFee) is evaluated against totalAssets() before the mint, so the denominator excludes the newly minted fee shares, diluting the fee.
**Insight:** When minting fee or reward shares, price them against assets net of the fee (currentAssets - totalFee) or post-mint supply; using pre-mint totalAssets systematically undercharges because the mint itself dilutes share value.