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

Share Inflation Attack via Incorrect Deposit Accounting Leads to Vault Drain

ishaanbansal2312

Root + Impact

Description

NORMAL BEHAVIOUR:

  • In an ERC4626-compliant vault, when a user deposits assets, the contract should mint shares based on the actual amount of tokens received by the vault, ensuring that shares always reflect the vault’s asset backing.

PROBLEM:

  • The contract calculates minted shares before transferring tokens and assumes assets == received_tokens.
    This is incorrect and breaks ERC4626 share math.

  • If the underlying token charges a transfer fee (burn, reflection, treasury-tax), the vault receives less token value than expected, but still mints full shares, inflating share supply and allowing an attacker to drain the vault.

// Root cause in the codebase with @> marks to highlight the relevant section

Risk

Likelihood:

  • High – happens on every deposit with taxed-tokens, fee-on-transfer tokens, rebasing tokens, or even sync delays in balanceOf.

Impact:

  • High – allows attacker to mint more shares than deposited assets → drain vault.
    Affects all depositors, all payout logic, and final winner withdrawal.

Proof of Concept

Since there is some tax fee that gets burned while adding the assets to the token and minting shares for it but it still allots the same number of shares for the user which makes it calculate wrong and allot wrong payout to the winner making the logic flawed and unfair for other users

// Vault has 1000 tokens, 1000 shares.
​
user.deposit(100);
​
// Contract calculates shares as if 90 tokens will arrive:
participantShares = _convertToShares(90); // 90 shares
​
// But underlying token burns 10% on transferFrom:
vault receives = 81 tokens
​
// User gets 90 shares backed by 81 tokens
// 9 tokens of free value created
​
// Attacker repeats deposit multiple times - > drains vault by withdrawing inflated shares
​

Recommended Mitigation :

Transfer the assets first to the token and then convert and map shares for the user for correct implementation of logic and prevent funds drain and unfair result and payout.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
​
uint256 fee = _getParticipationFee(assets);
uint256 stakeAsset = assets - fee;
​
- uint256 participantShares = _convertToShares(stakeAsset);
+ // transfer first
+ uint256 prev = IERC20(asset()).balanceOf(address(this));
​
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
​
+ uint256 received = IERC20(asset()).balanceOf(address(this)) - prev;
+ uint256 participantShares = _convertToShares(received);
​
_mint(receiver, participantShares);
}
​
Updates

Appeal created

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

Fee on transfer tokens

Support

FAQs

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

Give us feedback!