fix(cash): keep a module's authority over its own pending withdrawal after de-whitelisting

src/interfaces/ICashModule.sol

@@ -143,6 +143,10 @@ struct SafeCashConfig {
     uint96 lendOptOutFinalizeTime;
     /// @notice True once the safe's borrow/collateral engine is the Aave gateway (set at onboarding or by migration; one-way)
     bool usesLendGateway;
+    /// @notice True while the pending withdrawal request was placed by its recipient module through requestWithdrawalByModule.
+    ///         Recorded per request, so the module keeps sole authority over that request after governance removes it from
+    ///         either whitelist (removal only blocks new requests). Appended last: packs into the slot above.
+    bool withdrawalRequestedByModule;
 }
 
 /**

src/modules/cash/CashLendLib.sol

@@ -56,6 +56,10 @@ library CashLendLib {
     /// @dev Pad on resupply sizing so Aave share rounding cannot leave the headroom short; excess supply is harmless
     uint256 internal constant RESUPPLY_BUFFER_BPS = 10;
 
+    /// @dev Gas cap on cancelBridgeByCashModule for a module governance removed (see _notifyRemovedModule). The
+    ///      in-repo callbacks only clear the module's own record for the safe, well under this cap.
+    uint256 internal constant REMOVED_MODULE_CALLBACK_GAS = 200_000;
+
     // Threads a debit spend's cross-token state through the sizing pass. Bundled into one struct so the
     // sizing frames stay well under the legacy stack limit (the price provider is a parameter here, where in
     // the module it was a free immutable).
@@ -92,10 +96,29 @@ library CashLendLib {
         return dispatcher;
     }
 
+    /**
+     * @notice Whether the safe's pending withdrawal belongs to its recipient module, which alone may then
+     *         process or cancel it
+     * @dev A request placed through requestWithdrawalByModule is recorded as the module's own and stays so
+     *      after governance removes the module from either whitelist: removal only blocks new requests. A
+     *      request without that record (placed by the owners, or by a module before the record existed)
+     *      belongs to a module only while its recipient is on the withdraw-requester list, as before.
+     * @param $ The CashModule storage (passed by the delegatecalling module)
+     * @param safe Address of the EtherFi Safe
+     * @return True if only the request's recipient module may process or cancel it
+     */
+    function isModuleWithdrawal(CashModuleStorageContract.CashModuleStorage storage $, address safe) internal view returns (bool) {
+        SafeCashConfig storage safeCashConfig = $.safeCashConfig[safe];
+        return safeCashConfig.withdrawalRequestedByModule || $.whitelistedModulesCanRequestWithdraw.contains(safeCashConfig.pendingWithdrawalRequest.recipient);
+    }
+
     /**
      * @notice Cancels the safe's pending withdrawal request, if any, and emits WithdrawalCancelled
      * @dev The module's internal _cancelOldWithdrawal delegates here; the spend paths in this library call
-     *      it directly when a spend wins the competing claim over a reservation.
+     *      it directly when a spend wins the competing claim over a reservation. The recipient module hears
+     *      of the cancel through cancelBridgeByCashModule: a module on both whitelists exactly as before (its
+     *      revert reverts the cancel), and a module governance has since removed whenever the request is
+     *      recorded as its own, through _notifyRemovedModule so it cannot block the cancel.
      * @param $ The CashModule storage (passed by the delegatecalling module)
      * @param dataProvider The module's data provider (to vet the recipient before the bridge-cancel call)
      * @param safe Address of the EtherFi Safe
@@ -105,13 +128,34 @@ library CashLendLib {
         address recipient = safeCashConfig.pendingWithdrawalRequest.recipient;
 
         if (safeCashConfig.pendingWithdrawalRequest.tokens.length > 0) {
-            if ($.whitelistedModulesCanRequestWithdraw.contains(recipient)) {
-                // Only call the function if the module is whitelisted on data provider
-                if (dataProvider.isWhitelistedModule(recipient)) IBridgeModule(recipient).cancelBridgeByCashModule(safe);
+            if ($.whitelistedModulesCanRequestWithdraw.contains(recipient) && dataProvider.isWhitelistedModule(recipient)) {
+                IBridgeModule(recipient).cancelBridgeByCashModule(safe);
+            } else if (safeCashConfig.withdrawalRequestedByModule) {
+                _notifyRemovedModule(recipient, safe);
             }
 
             $.cashEventEmitter.emitWithdrawalCancelled(safe, safeCashConfig.pendingWithdrawalRequest.tokens, safeCashConfig.pendingWithdrawalRequest.amounts, recipient);
             delete safeCashConfig.pendingWithdrawalRequest;
+            delete safeCashConfig.withdrawalRequestedByModule;
+        }
+    }
+
+    /**
+     * @dev Tells a module that governance removed from a whitelist that CashModule cancelled its request, without
+     *      letting it block the cancel: spends, liquidations (preLiquidate) and the owners' replacement request all
+     *      cancel through here, and the removed module may be broken or retired to a reverting implementation. The
+     *      call gets at most REMOVED_MODULE_CALLBACK_GAS, its result is ignored and no return data is copied, so a
+     *      reverting, gas-burning or return-bombing module cannot fail the cancel. Removal therefore no longer stops
+     *      the callback from running (it may re-enter CashModule with the authority the module already holds over
+     *      its own request); it only stops it from blocking. A module that needs the signal must keep its callback
+     *      within that gas.
+     * @param module The removed module that requested the withdrawal (its recipient)
+     * @param safe Address of the EtherFi Safe
+     */
+    function _notifyRemovedModule(address module, address safe) private {
+        bytes memory data = abi.encodeCall(IBridgeModule.cancelBridgeByCashModule, (safe));
+        assembly ("memory-safe") {
+            pop(call(REMOVED_MODULE_CALLBACK_GAS, module, 0, add(data, 0x20), mload(data), 0, 0))
         }
     }
 

src/modules/cash/CashModuleSetters.sol

@@ -257,7 +257,7 @@ contract CashModuleSetters is CashModuleStorageContract {
         // from loose balances instead of pulling from the lend market
         CashLendLib.processLendOptOutIfReady($, safe);
 
-        _requestWithdrawal(safe, tokens, amounts, recipient, $.withdrawalDelay);
+        _requestWithdrawal(safe, tokens, amounts, recipient, $.withdrawalDelay, false);
     }
 
     /**
@@ -284,7 +284,7 @@ contract CashModuleSetters is CashModuleStorageContract {
 
         address recipient = msg.sender; // The module itself is the recipient
 
-        _requestWithdrawal(safe, tokens, amounts, recipient, _withdrawalDelayForModule($, msg.sender));
+        _requestWithdrawal(safe, tokens, amounts, recipient, _withdrawalDelayForModule($, msg.sender), true);
     }
 
     /**
@@ -339,25 +339,28 @@ contract CashModuleSetters is CashModuleStorageContract {
         if ($$.pendingWithdrawalRequest.tokens.length == 0) revert WithdrawalDoesNotExist();
 
         // Ensure that the module that requested the withdrawal is the one cancelling it
-        if ($.whitelistedModulesCanRequestWithdraw.contains($$.pendingWithdrawalRequest.recipient)) revert InvalidWithdrawRequest();
+        if (CashLendLib.isModuleWithdrawal($, safe)) revert InvalidWithdrawRequest();
 
         CashVerificationLib.verifyCancelWithdrawalSig(safe, IEtherFiSafe(safe).useNonce(), signers, signatures);
         _cancelOldWithdrawal(safe);
     }
 
     /**
      * @notice Cancels a pending withdrawal request by the module
-     * @dev Only callable by whitelisted modules that requested the withdrawal
+     * @dev Only callable by the module that requested the withdrawal, including after governance removed it from
+     *      either whitelist
      * @param safe Address of the EtherFi Safe
      */
     function cancelWithdrawalByModule(address safe) external onlyEtherFiSafe(safe) {
         CashModuleStorage storage $ = _getCashModuleStorage();
         SafeCashConfig storage $$ = $.safeCashConfig[safe];
 
         if ($$.pendingWithdrawalRequest.tokens.length == 0) revert WithdrawalDoesNotExist();
-        if (!$.whitelistedModulesCanRequestWithdraw.contains($$.pendingWithdrawalRequest.recipient)) revert InvalidWithdrawRequest();
+        if (!CashLendLib.isModuleWithdrawal($, safe)) revert InvalidWithdrawRequest();
         if (msg.sender != $$.pendingWithdrawalRequest.recipient) revert OnlyModuleThatRequestedCanCancel();
-        if (!etherFiDataProvider.isWhitelistedModule(msg.sender)) revert ModuleNotWhitelistedOnDataProvider();
+        // A request recorded as the module's own stays cancellable by it after governance removes the module;
+        // one without the record still needs the module on the DataProvider whitelist, as before
+        if (!$$.withdrawalRequestedByModule && !etherFiDataProvider.isWhitelistedModule(msg.sender)) revert ModuleNotWhitelistedOnDataProvider();
 
         _cancelOldWithdrawal(safe);
     }
@@ -420,11 +423,12 @@ contract CashModuleSetters is CashModuleStorageContract {
      * @param amounts Array of token amounts to withdraw
      * @param recipient Address to receive the withdrawn tokens
      * @param withdrawalDelay Delay to apply to this request
+     * @param requestedByModule True when the recipient module places the request (requestWithdrawalByModule)
      * @custom:throws RecipientCannotBeAddressZero if recipient is the zero address
      * @custom:throws InvalidInput if tokens is empty
      * @custom:throws AmountZero if every amount is zero
      */
-    function _requestWithdrawal(address safe, address[] memory tokens, uint256[] memory amounts, address recipient, uint64 withdrawalDelay) internal {
+    function _requestWithdrawal(address safe, address[] memory tokens, uint256[] memory amounts, address recipient, uint64 withdrawalDelay, bool requestedByModule) internal {
         CashModuleStorage storage $ = _getCashModuleStorage();
         SafeCashConfig storage $$ = $.safeCashConfig[safe];
 
@@ -446,6 +450,7 @@ contract CashModuleSetters is CashModuleStorageContract {
         _checkBalance(safe, tokens, amounts);
 
         $$.pendingWithdrawalRequest = WithdrawalRequest({ tokens: tokens, amounts: amounts, recipient: recipient, finalizeTime: finalTime });
+        $$.withdrawalRequestedByModule = requestedByModule;
         $.cashEventEmitter.emitWithdrawalRequested(safe, tokens, amounts, recipient, finalTime);
 
         // Legacy safes only: their loose tokens are DebtManager collateral, so a withdrawal can worsen the

src/modules/cash/CashModuleStorageContract.sol

@@ -282,8 +282,8 @@ contract CashModuleStorageContract is UpgradeableProxy, ModuleBase {
         if ($$.pendingWithdrawalRequest.tokens.length == 0) revert WithdrawalDoesNotExist();
 
         if ($$.pendingWithdrawalRequest.finalizeTime > block.timestamp) revert CannotWithdrawYet();
-        // Ensure that if the recipient of withdrawal is a whitelisted withdrawal module, only that module can process the withdrawal
-        if ($.whitelistedModulesCanRequestWithdraw.contains($$.pendingWithdrawalRequest.recipient)) {
+        // Ensure that a module's withdrawal is processed only by that module, also after governance removes it from either whitelist
+        if (CashLendLib.isModuleWithdrawal($, safe)) {
             if (msg.sender != $$.pendingWithdrawalRequest.recipient) revert OnlyModuleThatRequestedCanWithdraw();
         }
         _areAssetsWithdrawable($, $$.pendingWithdrawalRequest.tokens);
@@ -320,6 +320,7 @@ contract CashModuleStorageContract is UpgradeableProxy, ModuleBase {
         _getCashModuleStorage().cashEventEmitter.emitWithdrawalProcessed(safe, $$.pendingWithdrawalRequest.tokens, $$.pendingWithdrawalRequest.amounts, recipient);
 
         delete $$.pendingWithdrawalRequest;
+        delete $$.withdrawalRequestedByModule;
 
         // Legacy safes only: see _requestWithdrawal. Gateway safes are health-gated by Aave at request time
         // and would revert here if DebtManager cannot price a supplied Aave asset.