Fix reentrancy effort-limit bypass in FlowTransactionScheduler

contracts/FlowTransactionScheduler.cdc

@@ -795,6 +795,9 @@ access(all) contract FlowTransactionScheduler {
             // Deposit the fees to the service account's vault
             FlowTransactionScheduler.depositFees(from: <-fees)
 
+            // Reserve the slot capacity before invoking handler callback.
+            self.addTransaction(slot: estimate.timestamp!, txData: transactionData)
+
             let handlerRef = handlerCap.borrow()
                 ?? panic("Invalid transaction handler: Could not borrow a reference to the transaction handler")
 
@@ -812,9 +815,6 @@ access(all) contract FlowTransactionScheduler {
                 transactionHandlerPublicPath: handlerPublicPath
             )
 
-            // Add the transaction to the slot queue and update the internal state
-            self.addTransaction(slot: estimate.timestamp!, txData: transactionData)
-
             return <-create ScheduledTransaction(
                 id: transactionID, 
                 timestamp: estimate.timestamp!,
@@ -1010,7 +1010,19 @@ access(all) contract FlowTransactionScheduler {
             // and the cumulative slot total is bounded by priorityEffortLimit[priority] which is
             // checked on every schedule() call. UInt64 max (~1.8e19) far exceeds any reachable sum.
             let slotEfforts = &self.slotUsedEffort[slot]! as auth(Mutate) &{Priority: UInt64}
-            slotEfforts[txData.priority] = slotEfforts[txData.priority]! + txData.executionEffort
+            let newSlotEffort = slotEfforts[txData.priority]! + txData.executionEffort
+
+            // The per-priority slot total must never exceed the configured limit.
+            // estimate() already enforces this before schedule() reserves capacity, so
+            // this assertion should be unreachable; it guards against any reentrancy vector
+            // that reaches addTransaction() without a fresh capacity check.
+            let limit = self.config.priorityEffortLimit[txData.priority]!
+            assert(
+                newSlotEffort <= limit,
+                message: "Invalid execution effort: scheduling \(txData.executionEffort) would raise the \(txData.priority.rawValue) priority slot total to \(newSlotEffort), exceeding the limit of \(limit)."
+            )
+
+            slotEfforts[txData.priority] = newSlotEffort
 
             // Store the transaction in the transactions map
             self.transactions[txData.id] = txData

contracts/testContracts/TestFlowScheduledTransactionHandler.cdc

@@ -93,6 +93,89 @@ access(all) contract TestFlowScheduledTransactionHandler {
         return <- create Handler(name: "Test FlowTransactionHandler Resource", description: "Executes a variety of transactions for different test cases")
     }
 
+    /** ---------------------------------------------------------------------------------
+     Reentrant handler for testing the reentrancy protection in FlowTransactionScheduler.schedule()
+     --------------------------------------------------------------------------------- */
+
+    access(all) let ReentrantHandlerStoragePath: StoragePath
+    access(all) let ReentrantHandlerPublicPath: PublicPath
+
+    /// The timestamp that the reentrant schedule attempt targets.
+    /// A constant far-future timestamp is used so tests can schedule the outer transaction
+    /// at the exact same slot without depending on the block time.
+    access(all) let reentrantScheduleTimestamp: UFix64
+
+    /// The execution effort used for the reentrant schedule attempt
+    access(all) let reentrantScheduleEffort: UInt64
+
+    /// Whether the reentrant schedule has already been attempted.
+    /// Used to only attempt the reentrant schedule once to avoid infinite recursion.
+    access(all) var reentrantScheduleAttempted: Bool
+
+    /// ReentrantHandler is a malicious handler used to test that schedule() cannot be
+    /// reentered to bypass the per-slot effort limit.
+    /// Its resolveView() attempts to schedule another high-priority transaction for the
+    /// same slot while the outer schedule() call is still in flight.
+    access(all) resource ReentrantHandler: FlowTransactionScheduler.TransactionHandler {
+
+        access(all) view fun getViews(): [Type] {
+            return [Type<StoragePath>(), Type<PublicPath>()]
+        }
+
+        access(all) fun resolveView(_ view: Type): AnyStruct? {
+            switch view {
+                case Type<StoragePath>():
+                    return TestFlowScheduledTransactionHandler.ReentrantHandlerStoragePath
+                case Type<PublicPath>():
+                    // Malicious behavior: attempt to reenter schedule() while the outer
+                    // schedule() call that triggered this resolveView() is still in flight.
+                    // Only attempt once to avoid infinite recursion.
+                    if !TestFlowScheduledTransactionHandler.reentrantScheduleAttempted {
+                        TestFlowScheduledTransactionHandler.reentrantScheduleAttempted = true
+                        TestFlowScheduledTransactionHandler.attemptReentrantSchedule()
+                    }
+                    return TestFlowScheduledTransactionHandler.ReentrantHandlerPublicPath
+                default:
+                    return nil
+            }
+        }
+
+        access(FlowTransactionScheduler.Execute)
+        fun executeTransaction(id: UInt64, data: AnyStruct?) {
+            // The reentrant transactions are never meant to be executed in tests
+        }
+    }
+
+    access(all) fun createReentrantHandler(): @ReentrantHandler {
+        return <- create ReentrantHandler()
+    }
+
+    /// Attempts to schedule a high-priority transaction for the same slot as the
+    /// outer schedule() call that is currently in flight.
+    /// With the reentrancy fix in place, the outer call has already reserved its slot
+    /// capacity, so this nested schedule fails the effort estimate and panics.
+    access(contract) fun attemptReentrantSchedule() {
+        let manager = self.borrowManager()
+
+        // Find the entitled capability for the reentrant handler
+        var handlerCap: Capability<auth(FlowTransactionScheduler.Execute) &{FlowTransactionScheduler.TransactionHandler}>? = nil
+        for controller in self.account.capabilities.storage.getControllers(forPath: self.ReentrantHandlerStoragePath) {
+            if let cap = controller.capability as? Capability<auth(FlowTransactionScheduler.Execute) &{FlowTransactionScheduler.TransactionHandler}> {
+                handlerCap = cap
+                break
+            }
+        }
+
+        manager.schedule(
+            handlerCap: handlerCap ?? panic("Could not find an entitled capability for the reentrant transaction handler"),
+            data: nil,
+            timestamp: self.reentrantScheduleTimestamp,
+            priority: FlowTransactionScheduler.Priority.High,
+            executionEffort: self.reentrantScheduleEffort,
+            fees: <-self.getFeeFromVault(amount: 10.0)
+        )
+    }
+
     access(all) fun getSucceededTransactions(): [UInt64] {
         return self.succeededTransactions
     }
@@ -115,5 +198,11 @@ access(all) contract TestFlowScheduledTransactionHandler {
 
         self.HandlerStoragePath = /storage/testTransactionHandler
         self.HandlerPublicPath = /public/testTransactionHandler
+
+        self.ReentrantHandlerStoragePath = /storage/testReentrantTransactionHandler
+        self.ReentrantHandlerPublicPath = /public/testReentrantTransactionHandler
+        self.reentrantScheduleTimestamp = 4_000_000_000.0
+        self.reentrantScheduleEffort = 9_000
+        self.reentrantScheduleAttempted = false
     }
 } 

tests/scheduled_transaction_test_helpers.cdc

@@ -82,6 +82,38 @@ access(all) fun scheduleTransaction(
     }
 }
 
+// Helper function for scheduling a transaction with the malicious reentrant handler
+// whose resolveView() attempts to reenter schedule() while the outer call is in flight
+access(all) fun scheduleReentrantTransaction(
+    timestamp: UFix64,
+    fee: UFix64,
+    effort: UInt64,
+    priority: UInt8,
+    testName: String,
+    failWithErr: String?
+) {
+    var tx = Test.Transaction(
+        code: Test.readFile("../transactions/transactionScheduler/schedule_transaction_reentrant.cdc"),
+        authorizers: [admin.address],
+        signers: [admin],
+        arguments: [timestamp, fee, effort, priority],
+    )
+    var result = Test.executeTransaction(tx)
+
+    if let error = failWithErr {
+        Test.expect(result, Test.beFailed())
+        Test.assertError(
+            result,
+            errorMessage: error
+        )
+
+    } else {
+        if result.error != nil {
+            Test.assert(result.error == nil, message: "Transaction failed with error: \(result.error!.message) for test case: \(testName)")
+        }
+    }
+}
+
 access(all) fun scheduleTransactionByHandler(
     handlerTypeIdentifier: String,
     handlerUUID: UInt64?,

tests/transactionScheduler_test.cdc

@@ -67,6 +67,35 @@ access(all) fun testInit() {
     Test.assertEqual(lowPriorityMaxEffort, effort)
 }
 
+// Tests that schedule() cannot be reentered from a malicious handler's resolveView()
+// to bypass the per-slot effort limit for a priority.
+// The reentrant handler attempts to schedule another high-priority transaction for the
+// same slot while the outer schedule() call is still in flight.
+// Since the outer call reserves its slot capacity before invoking the handler callback,
+// the nested schedule() fails the effort estimate, panics, and the whole transaction
+// is rolled back atomically.
+access(all) fun testScheduleReentrancy() {
+
+    let timestamp = TestFlowScheduledTransactionHandler.reentrantScheduleTimestamp
+    let reentrantEffort = TestFlowScheduledTransactionHandler.reentrantScheduleEffort
+
+    // The outer transaction requests an effort that fits in the slot on its own,
+    // but would exceed the high priority slot limit if the reentrant schedule succeeded
+    scheduleReentrantTransaction(
+        timestamp: timestamp,
+        fee: feeAmount,
+        effort: reentrantEffort,
+        priority: highPriority,
+        testName: "Schedule with a handler that reenters schedule()",
+        failWithErr: "Invalid execution effort: \(reentrantEffort) is greater than the priority's available effort for the requested timestamp."
+    )
+
+    // The failed schedule must have been rolled back atomically,
+    // so the slot must still have all its effort available
+    let effort = getSlotAvailableEffort(timestamp: timestamp, priority: highPriority)
+    Test.assertEqual(highPriorityMaxEffort, effort)
+}
+
 access(all) fun testGetSizeOfData() {
 
     // Test different values for data to verify that it reports the correct sizes