From e5eaefafe7b7b9f07209f622d99d3b61c2c733ef Mon Sep 17 00:00:00 2001
From: Amine Alami <43780877+Alami-Amine@users.noreply.github.com>
Date: Mon, 30 Mar 2026 14:38:39 +0200
Subject: [PATCH] [IM Engine] add missing nullptr checks for
 IterateSubscriptions (#43625)

* [IM Engine] add missing nullptr checks for IterateSubscriptions

* Adding unit tests

* integrate copilot comments

* test adapt to integrated comment

* Add ChipErrorLog when failed to allocate iterator

(cherry picked from commit dfcd5cea81913ea9d53611eb05b38475433723fb)
---
 src/app/InteractionModelEngine.cpp           | 19 ++++++--
 src/app/SubscriptionResumptionStorage.h      |  3 ++
 src/app/tests/TestInteractionModelEngine.cpp | 51 ++++++++++++++++++++
 3 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
index e1488e6cdaf8b3..0d439025ce21d0 100644
--- a/src/app/InteractionModelEngine.cpp
+++ b/src/app/InteractionModelEngine.cpp
@@ -168,8 +168,15 @@ class AutoReleaseSubscriptionInfoIterator
 {
 public:
     AutoReleaseSubscriptionInfoIterator(SubscriptionResumptionStorage::SubscriptionInfoIterator * iterator) : mIterator(iterator){};
-    ~AutoReleaseSubscriptionInfoIterator() { mIterator->Release(); }
+    ~AutoReleaseSubscriptionInfoIterator()
+    {
+        if (mIterator)
+        {
+            mIterator->Release();
+        }
+    }
 
+    explicit operator bool() const { return mIterator != nullptr; }
     SubscriptionResumptionStorage::SubscriptionInfoIterator * operator->() const { return mIterator; }
 
 private:
@@ -777,6 +784,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest
             {
                 SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
                 auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
+                VerifyOrReturnError(nullptr != iterator, Status::ResourceExhausted);
 
                 while (iterator->Next(subscriptionInfo))
                 {
@@ -2098,9 +2106,11 @@ CHIP_ERROR InteractionModelEngine::ResumeSubscriptions()
     // future improvements: https://github.com/project-chip/connectedhomeip/issues/25439
 
     SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
-    auto * iterator             = mpSubscriptionResumptionStorage->IterateSubscriptions();
     mNumOfSubscriptionsToResume = 0;
     uint16_t minInterval        = 0;
+
+    auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
+    VerifyOrReturnError(nullptr != iterator, CHIP_ERROR_NO_MEMORY);
     while (iterator->Next(subscriptionInfo))
     {
         mNumOfSubscriptionsToResume++;
@@ -2137,6 +2147,7 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap
 #endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
     AutoReleaseSubscriptionInfoIterator iterator(imEngine->mpSubscriptionResumptionStorage->IterateSubscriptions());
+    VerifyOrReturn(iterator, ChipLogError(InteractionModel, "Failed to allocate subscription resumption iterator"));
     while (iterator->Next(subscriptionInfo))
     {
         // If subscription happens between reboot and this timer callback, it's already live and should skip resumption
@@ -2209,7 +2220,9 @@ bool InteractionModelEngine::HasSubscriptionsToResume()
 
     // Look through persisted subscriptions and see if any aren't already in mReadHandlers pool
     SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
-    auto * iterator                = mpSubscriptionResumptionStorage->IterateSubscriptions();
+    auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
+    // Conservatively assume there may be subscriptions to resume if we cannot get an iterator.
+    VerifyOrReturnValue(iterator != nullptr, true);
     bool foundSubscriptionToResume = false;
     while (iterator->Next(subscriptionInfo))
     {
diff --git a/src/app/SubscriptionResumptionStorage.h b/src/app/SubscriptionResumptionStorage.h
index 23a06c91e3bced..e95dc409b839c8 100644
--- a/src/app/SubscriptionResumptionStorage.h
+++ b/src/app/SubscriptionResumptionStorage.h
@@ -141,6 +141,9 @@ class SubscriptionResumptionStorage
      * Iterate through persisted subscriptions
      *
      * @return A valid iterator on success. Use CommonIterator accessor to retrieve SubscriptionInfo
+     *
+     * Warning: Implementations using fixed-size pools may return nullptr if the iterator pool is exhausted.
+     * Callers should check for nullptr before using the iterator.
      */
     virtual SubscriptionInfoIterator * IterateSubscriptions() = 0;
 
diff --git a/src/app/tests/TestInteractionModelEngine.cpp b/src/app/tests/TestInteractionModelEngine.cpp
index d09f5647d6e275..f2c111e592f8fb 100644
--- a/src/app/tests/TestInteractionModelEngine.cpp
+++ b/src/app/tests/TestInteractionModelEngine.cpp
@@ -68,6 +68,7 @@ class TestInteractionModelEngine : public chip::Test::AppContext
     void TestSubjectHasActiveSubscriptionSubWithCAT();
     void TestSubscriptionResumptionTimer();
     void TestDecrementNumSubscriptionsToResume();
+    void TestHasSubscriptionsToResumeHandlesNullIterator();
     void TestFabricHasAtLeastOneActiveSubscription();
     void TestFabricHasAtLeastOneActiveSubscriptionWithMixedStates();
     static int GetAttributePathListLength(SingleLinkedListNode<AttributePathParams> * apattributePathParamsList);
@@ -830,5 +831,55 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestFabricHasAtLeastOneActiveSub
     EXPECT_FALSE(engine->FabricHasAtLeastOneActiveSubscription(fabricIndex));
 }
 
+#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
+
+// Mock SubscriptionResumptionStorage that always returns nullptr from IterateSubscriptions(). Used to test null-check handling in
+// the InteractionModelEngine.
+class NullIteratorSubscriptionResumptionStorage : public chip::app::SubscriptionResumptionStorage
+{
+public:
+    SubscriptionInfoIterator * IterateSubscriptions() override { return nullptr; }
+    CHIP_ERROR Save(SubscriptionInfo & subscriptionInfo) override { return CHIP_NO_ERROR; }
+    CHIP_ERROR Delete(chip::NodeId nodeId, chip::FabricIndex fabricIndex, chip::SubscriptionId subscriptionId) override
+    {
+        return CHIP_NO_ERROR;
+    }
+    CHIP_ERROR DeleteAll(chip::FabricIndex fabricIndex) override { return CHIP_NO_ERROR; }
+};
+
+// Test verifies that ResumeSubscriptions handles a nullptr iterator gracefully by returning CHIP_ERROR_NO_MEMORY and not crashing
+TEST_F(TestInteractionModelEngine, TestResumeSubscriptionsHandlesNullIterators)
+{
+    InteractionModelEngine * engine = InteractionModelEngine::GetInstance();
+
+    NullIteratorSubscriptionResumptionStorage nullIteratorStorage;
+
+    engine->SetDataModelProvider(CodegenDataModelProviderInstance(nullptr /* delegate */));
+    EXPECT_EQ(CHIP_NO_ERROR,
+              engine->Init(&GetExchangeManager(), &GetFabricTable(), app::reporting::GetDefaultReportScheduler(), nullptr,
+                           &nullIteratorStorage));
+
+    EXPECT_EQ(engine->ResumeSubscriptions(), CHIP_ERROR_NO_MEMORY);
+}
+
+#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+
+// Test verifies that HasSubscriptionsToResume handles a nullptr iterator gracefully by returning true and not crashing.
+TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestHasSubscriptionsToResumeHandlesNullIterator)
+{
+    InteractionModelEngine * engine = InteractionModelEngine::GetInstance();
+
+    NullIteratorSubscriptionResumptionStorage nullIteratorStorage;
+
+    engine->SetDataModelProvider(CodegenDataModelProviderInstance(nullptr /* delegate */));
+    EXPECT_EQ(CHIP_NO_ERROR,
+              engine->Init(&GetExchangeManager(), &GetFabricTable(), app::reporting::GetDefaultReportScheduler(), nullptr,
+                           &nullIteratorStorage));
+
+    EXPECT_TRUE(engine->HasSubscriptionsToResume());
+}
+#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
+
 } // namespace app
 } // namespace chip