Fix ERC-4626 Vault Exploits Before They Cost You Millions - Security Audit Guide 2025

// Personal note: I found this in 3 production vaults after "successful" audits
// Test if first depositor can be exploited by inflation attack

function testInflationAttack() public {
    // Attacker deposits 1 wei to mint initial shares
    vm.startPrank(attacker);
    asset.approve(address(vault), 1);
    vault.deposit(1, attacker);
    
    // Attacker donates large amount directly to vault (not via deposit)
    asset.transfer(address(vault), 1e18); // 1 token donation
    vm.stopPrank();
    
    // Victim deposits normal amount
    vm.startPrank(victim);
    asset.approve(address(vault), 1000e18);
    uint256 shares = vault.deposit(1000e18, victim);
    vm.stopPrank();
    
    // Watch out: If shares rounds to 0 or victim gets way fewer shares, vault is vulnerable
    assertGt(shares, 0, "Victim got 0 shares - CRITICAL BUG");
    assertGt(shares, 999e18, "Victim lost value to rounding - CRITICAL BUG");
}

// Add virtual shares offset to prevent inflation attacks
uint256 internal constant INITIAL_SHARE_OFFSET = 1e8;

function _convertToShares(uint256 assets, Math.Rounding rounding) 
    internal view virtual returns (uint256) {
    uint256 supply = totalSupply() + INITIAL_SHARE_OFFSET;
    return (assets == 0 || supply == 0)
        ? assets
        : assets.mulDiv(supply, totalAssets() + 1, rounding);
}

// This line saved me 8 hours of debugging - test with token that has callbacks
// Using a malicious ERC-777 token to test reentrancy

contract MaliciousToken is ERC20 {
    address public vault;
    bool public attackExecuted;
    
    function setVault(address _vault) external {
        vault = _vault;
    }
    
    // Hook that fires on transfer - attempts reentry
    function _afterTokenTransfer(address from, address to, uint256 amount) 
        internal override {
        if (to == vault && !attackExecuted) {
            attackExecuted = true;
            // Try to withdraw while deposit is still executing
            IVault(vault).withdraw(amount, address(this), address(this));
        }
    }
}

function testReentrancyOnDeposit() public {
    MaliciousToken malToken = new MaliciousToken();
    Vault testVault = new Vault(address(malToken));
    malToken.setVault(address(testVault));
    
    // Don't skip this validation - learned the hard way
    vm.expectRevert(); // Should revert if protected properly
    testVault.deposit(1000e18, address(this));
}

import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";

contract SecureVault is ERC4626, ReentrancyGuard {
    // Apply to deposit, mint, withdraw, AND redeem
    function deposit(uint256 assets, address receiver) 
        public virtual override nonReentrant returns (uint256) {
        // CRITICAL: Update state before external calls
        uint256 shares = previewDeposit(assets);
        _mint(receiver, shares);
        
        // External call comes AFTER state changes
        SafeERC20.safeTransferFrom(asset, msg.sender, address(this), assets);
        return shares;
    }
}

// Test every conversion function for correct rounding
function testRoundingDirection() public {
    // Setup: Create scenario where rounding matters
    asset.mint(address(this), 1000e18);
    asset.approve(address(vault), 1000e18);
    vault.deposit(1000e18, address(this));
    
    // Donate 1 wei to create rounding scenario
    asset.mint(address(vault), 1);
    
    // Test deposit preview rounds DOWN (user gets fewer shares = vault wins)
    uint256 assetsToDeposit = 100e18;
    uint256 sharesPreview = vault.previewDeposit(assetsToDeposit);
    uint256 sharesActual = vault.deposit(assetsToDeposit, address(this));
    assertGe(sharesActual, sharesPreview, "Deposit rounding wrong - users lose extra");
    
    // Test redeem preview rounds UP (user needs more assets = vault wins)
    uint256 sharesToRedeem = 50e18;
    uint256 assetsPreview = vault.previewRedeem(sharesToRedeem);
    uint256 assetsActual = vault.redeem(sharesToRedeem, address(this), address(this));
    assertLe(assetsActual, assetsPreview, "Redeem rounding wrong - vault loses");
}

// WRONG - This will get exploited via sandwich attacks
function previewRedeem(uint256 shares) public view returns (uint256) {
    return shares.mulDiv(totalAssets(), totalSupply()); // Missing rounding param!
}

// CORRECT - Always specify rounding direction
function previewRedeem(uint256 shares) public view returns (uint256) {
    return shares.mulDiv(totalAssets(), totalSupply(), Math.Rounding.Up);
}

function testMaxDepositWhenPaused() public {
    // Pause the vault (if pausable)
    vault.pause();
    
    // Should return 0, not revert or return wrong value
    uint256 maxDep = vault.maxDeposit(address(this));
    assertEq(maxDep, 0, "maxDeposit should return 0 when paused");
    
    // Actual deposit should revert
    vm.expectRevert();
    vault.deposit(1e18, address(this));
}

function testMaxMintWhenCapReached() public {
    // If vault has deposit cap, test behavior at cap
    uint256 cap = vault.depositCap();
    asset.mint(address(this), cap);
    asset.approve(address(vault), cap);
    vault.deposit(cap, address(this));
    
    // Should return 0 once cap reached
    assertEq(vault.maxDeposit(address(this)), 0, "Should return 0 at cap");
    assertEq(vault.maxMint(address(this)), 0, "Should return 0 at cap");
}

// Test with fee-on-transfer token
contract FeeToken is ERC20 {
    uint256 public feePercent = 1; // 1% fee
    
    function _transfer(address from, address to, uint256 amount) internal override {
        uint256 fee = amount * feePercent / 100;
        super._transfer(from, to, amount - fee);
        super._transfer(from, address(0xdead), fee); // Burn fee
    }
}

function testFeeOnTransferToken() public {
    FeeToken feeToken = new FeeToken();
    Vault feeVault = new Vault(address(feeToken));
    
    feeToken.mint(address(this), 1000e18);
    feeToken.approve(address(feeVault), 1000e18);
    
    // Deposit 1000 tokens but only 990 arrive (1% fee)
    uint256 shares = feeVault.deposit(1000e18, address(this));
    
    // CRITICAL: Check vault accounting matches actual received amount
    uint256 actualBalance = feeToken.balanceOf(address(feeVault));
    uint256 expectedAssets = feeVault.totalAssets();
    
    // Vault is broken if these don't match - it's tracking wrong balance
    assertEq(actualBalance, expectedAssets, "Accounting broken with fee token");
}

function deposit(uint256 assets, address receiver) public returns (uint256) {
    // Check actual received amount, not parameter amount
    uint256 balanceBefore = asset.balanceOf(address(this));
    SafeERC20.safeTransferFrom(asset, msg.sender, address(this), assets);
    uint256 balanceAfter = asset.balanceOf(address(this));
    
    // Use actual received amount for share calculation
    uint256 actualAssets = balanceAfter - balanceBefore;
    uint256 shares = previewDeposit(actualAssets);
    _mint(receiver, shares);
    
    return shares;
}

function testPartialLiquidityWithdraw() public {
    // Setup: Deposit funds and deploy 80% to strategy
    asset.mint(address(this), 1000e18);
    asset.approve(address(vault), 1000e18);
    vault.deposit(1000e18, address(this));
    
    vault.deployToStrategy(800e18); // 80% deployed, 20% in vault
    
    // Try to withdraw 50% - should work with partial liquidity
    uint256 shares = vault.balanceOf(address(this));
    uint256 assetsReceived = vault.redeem(shares / 2, address(this), address(this));
    
    assertGt(assetsReceived, 0, "Should be able to withdraw with partial liquidity");
    
    // Try to withdraw more than available liquidity
    uint256 maxWithdraw = vault.maxWithdraw(address(this));
    assertLe(maxWithdraw, 200e18 + 1000, "maxWithdraw should reflect available liquidity");
}

function testSandwichAttackResistance() public {
    // Simulate profitable transaction that attackers would frontrun
    asset.mint(address(vault), 1000e18); // Vault receives profit
    
    // Attacker deposits large amount right before profit distribution
    vm.startPrank(attacker);
    asset.mint(attacker, 10000e18);
    asset.approve(address(vault), 10000e18);
    uint256 attackerShares = vault.deposit(10000e18, attacker);
    vm.stopPrank();
    
    // Original user deposits after attacker
    asset.mint(address(this), 1000e18);
    asset.approve(address(vault), 1000e18);
    uint256 userShares = vault.deposit(1000e18, address(this));
    
    // Check if attacker extracted unfair value
    uint256 attackerAssets = vault.previewRedeem(attackerShares);
    uint256 attackerProfit = attackerAssets - 10000e18;
    
    // If attacker made profit from this MEV, vault has issue
    assertLe(attackerProfit, 100e18, "Attacker extracted too much MEV value");
}