Thesauros Automated Interest Rebalancing Protocol Security Review Report

The Vault contract calculates totalAssets by aggregating the balances from all providers listed in the _providers array. However, the _withdraw function exclusively sources funds from the single designated activeProvider. This mismatch creates a scenario where withdrawals can fail despite the vault holding sufficient aggregate liquidity.

When the rebalance() function moves assets between providers without updating the activeProvider (when activateToProvider parameter is false), the active provider may retain insufficient funds to satisfy withdrawal requests. Users attempting to withdraw amounts that exceed the active provider's balance will experience transaction failures, even though maxWithdraw() indicates sufficient funds are available based on the total assets calculation.

Scenario:

1. The vault has two registered providers, ProviderA and ProviderB. ProviderA is the activeProvider.
2. The vault holds 1,000 USDC, all of which is deposited in ProviderA.
3. An operator executes a rebalance, moving 950 USDC from ProviderA to ProviderB, with the activateToProvider flag set to false.
4. The vault's totalAssets() function still correctly reports a balance of 1,000 USDC. However, the activeProvider (ProviderA) now holds only 50 USDC.
5. A user attempts to withdraw 100 USDC. The transaction fails because the _withdraw function attempts to pull funds from ProviderA, which has insufficient liquidity.

function rebalanceVault(
    IVault vault,
    uint256 assets,
    IProvider from,
    IProvider to,
    uint256 fee,
    bool activateToProvider
) external onlyExecutor returns (bool success) {
    uint256 assetsAtFrom = from.getDepositBalance(address(vault), vault);
 
    if (assets == type(uint256).max) {
        assets = assetsAtFrom;
    }
    if (assets == 0 || assets > assetsAtFrom) {
        revert VaultManager__InvalidAssetAmount();
    }
 
    vault.rebalance(assets, from, to, fee, activateToProvider);
 
    success = true;
}
 
function _getBalanceAtProviders()
    internal
    view
    returns (uint256 totalBalance)
{
    uint256 providerBalance;
    uint256 count = _providers.length;
    for (uint256 i; i < count; i++) {
        providerBalance = _providers[i].getDepositBalance(
            address(this),
            this
        );
        totalBalance += providerBalance;
    }
}

Align accounting and execution so that amounts ≤ maxWithdraw() never revert:

Option A: Implement multi-provider withdrawal in the vault. Attempt from activeProvider first; if insufficient, withdraw the shortfall from other providers sequentially until assets is met. Apply the same strategy for redeem.

Option B: Make limits liquidity-aware. Adjust maxWithdraw() (and maxRedeem() via convertToShares) to cap by the vault's immediately withdrawable liquidity under the current execution strategy.

In the Vault contract, the internal _deposit function uses _delegateActionToProvider to deposit the new assets to the provider protocol and the internal _withdraw function uses _delegateActionToProvider to withdraw the assets from the provider protocol.

The function _delegateActionToProvider is a delegate call to a provider that abstracts away the actual call to a supported protocol (Aave, Morpho, etc.). The external call is done before shares are minted in deposit and after shares are burned in withdraw: this means that there is an intermediate state where the share rate would be inflated from what it actually is.

This can be exploited to directly steal principal assets if one of the callbacks is reentrant. For example:

1. The vault has 200 WETH and 200 shares, 100 shares belong to user A.
2. User A withdraws 50 shares, calculated to 50 WETH.
3. The burn happens first, then the callback, so the vault temporarily has 200 WETH and 150 shares.
4. User A again withdraws another 50 shares inside of the callback, calculated to 66.67 WETH.
5. In the end, user A withdrew their 100 shares to 116.67 WETH. This obviously depends on the protocol that is being integrated with on whether there is the possibility of a callback, but this is an unknown. If for example an integrated project uses a dynamic swap to deposit/withdraw, that could lead into a pool with an attacker-controlled hook, it could lead to a callback.

There is therefore a heavy risk on the possibility of a callback and a critical dependency on one of the integrated protocols (and the protocol that they further depend on).

function _deposit(
    address caller,
    address receiver,
    uint256 assets,
    uint256 shares
) internal {
    _asset.safeTransferFrom(caller, address(this), assets);
    _delegateActionToProvider(assets, "deposit", activeProvider);
    _mint(receiver, shares);
 
    emit Deposit(caller, receiver, assets, shares);
}
 
function _withdraw(
    address caller,
    address receiver,
    address owner,
    uint256 assets,
    uint256 shares
) internal {
    uint256 withdrawFee = assets.mulDiv(
        withdrawFeePercent,
        PRECISION_FACTOR
    );
    uint256 assetsToReceiver = assets - withdrawFee;
 
    _burn(owner, shares);
    _delegateActionToProvider(assets, "withdraw", activeProvider);
 
    address _treasury = treasury;
 
    _asset.safeTransfer(_treasury, withdrawFee);
    _asset.safeTransfer(receiver, assetsToReceiver);
 
    emit FeeCharged(_treasury, assets, withdrawFee);
    emit Withdraw(caller, receiver, owner, assetsToReceiver, shares);
}

We recommend not to create risk and add a reentrancy guard to _deposit and _withdraw.

Any other contract that relies on the Vault's share rate (e.g. functions such as totalAssets, convertToShares, convertToAssets, etc.) could still be vulnerable to read-only reentrancy in the case of a 3rd party callback, but at least the direct risk to the Vault should be mitigated.

uint256 _maxWithdraw = maxWithdraw(owner);
if (assets > _maxWithdraw) {
    validatedAssets = _maxWithdraw;
    validatedShares = validatedAssets.mulDiv(shares, assets);
} else {
    validatedAssets = assets;
    validatedShares = shares;
}

When assets > _maxWithdraw, the process of recalculating validatedShares uses rounding CEIL when computing shares, but the code above uses FLOOR. As a result, validatedShares may end up being smaller than expected.

Consider checking that validatedShares and validatedAssets are not zero after the max-withdraw calculation. You can also combine this with ceiling rounding (Math.Rounding.Ceil) in mulDiv to reduce the chance of rounding down to zero.

if (assets > _maxWithdraw) {
    validatedAssets = _maxWithdraw;
    validatedShares = _convertToShares(validatedAssets, Math.Rounding.Ceil);

Aave offers incentives - such as staking or liquidity mining rewards (see: Incentives ) - to users who supply assets to the protocol. These rewards are typically distributed as additional tokens (e.g., AAVE or other governance tokens) and can be claimed by users through Aave's incentive mechanisms.

However, the current AaveV3Provider contract lacks functionality to claim these incentives. This omission prevents users from fully benefiting from their supplied assets on Aave.

The similar issue applies to Compound and Morpho, Dolomite either:

• Compound III Docs | Protocol Rewards
• Rewards on Morpho
• Token Mechanics | Dolomite

To resolve this, we should introduce a function that enables users to claim their Aave incentives. This would require integrating with Aave's Incentives Controller or Rewards Distributor contracts.

Commentary from the client:

Our protocol operates as an automated yield optimizer, and we have designed the system to capture only core lending yields.

The provider implementations use different accrual models when exposing yield/balance views:

• MorphoProvider computes rates using projected accrual and projected weights (via expectedMarketBalances/expectedSupplyAssets) and applies a wrapper fee.
• AaveV3Provider, CompoundV3Provider, and DolomiteProvider return values based on last‑updated protocol state/parameters without projection, which can be stale until a state‑changing transaction occurs. As a result, cross‑provider outputs are not directly comparable. This can skew off‑chain decisions (e.g., rebalancing) and, in mixed views, may introduce minor, transient drift when aggregating across providers.

Example scenario:

• A rebalancing routine compares providers using getDepositRate. Morpho's rate reflects projected accrual, while another provider returns a stale rate from hours earlier. The routine chooses a suboptimal destination and misses higher yield until the next update.

function _getMarketRate(
    MarketParams memory marketParams,
    Market memory market
) internal view returns (uint256 marketRate) {
    IMorpho morpho = _getMorpho();
 
    uint256 borrowRate;
    if (marketParams.irm == address(0)) {
        return 0;
    } else {
        borrowRate = IIrm(marketParams.irm)
            .borrowRateView(marketParams, market)
            .wTaylorCompounded(365 days);
    }
 
    (uint256 totalSupplyAssets, , uint256 totalBorrowAssets, ) =
        morpho.expectedMarketBalances(marketParams);
 
    uint256 utilization = totalBorrowAssets == 0
        ? 0
        : totalBorrowAssets.wDivUp(totalSupplyAssets);
 
    marketRate = borrowRate.wMulDown(1e18 - market.fee).wMulDown(
        utilization
    );
}

Standardize the accrual model across providers and implement it consistently in code:

• Either project accrual at read time for all providers before returning rates/balances, or
• Return only last‑updated values everywhere without projection.

Commentary from the client:

The different accrual models across providers reflect intentional design choices optimized for each protocol's specific mechanics.

The vault's core ERC4626 functions (deposit(), mint(), withdraw(), and redeem()) do not include slippage protection parameters, which could expose users to receiving unfavorable conversion rates in certain scenarios.

• What is protected: The vault reverts when a deposit would mint zero shares, preventing "0 shares" outcomes and principal loss on-chain. Deposits are also paused until setup completes and enforce a minimum asset amount (minAmount)
• What is not protected: The vault does not bound user‑expected outcomes for ERC4626 actions. All conversions rely on totalAssets() which aggregates provider balances. The vault's totalAssets() calculation relies on _getBalanceAtProviders(), which queries provider token balances (e.g., aTokens). This design could potentially be susceptible to manipulation through direct token transfers or through front-running of large transactions.

function totalAssets() public view override returns (uint256 assets) {
    return _getBalanceAtProviders();
}
 
function _getBalanceAtProviders()
    internal
    view
    returns (uint256 totalBalance)
{
    uint256 providerBalance;
    uint256 count = _providers.length;
    for (uint256 i; i < count; i++) {
        providerBalance = _providers[i].getDepositBalance(
            address(this),
            this
        );
        totalBalance += providerBalance;
    }
}

Consider adding optional slippage protection parameters to all four core ERC4626 functions. For deposit() and redeem(), add a minimum output parameter to ensure users receive at least the expected amount. For mint() and withdraw(), add a maximum input parameter to cap the amount users must provide. The functions should revert if the actual execution falls outside these user-defined bounds, providing defense-in-depth against both malicious front-running and unexpected state changes.

Commentary from the client:

Our protocol only accepts deposits from whitelisted addresses, which significantly reduces the attack surface for front-running and manipulation scenarios.