[IM]Fix leaked readClient in onFabricRemoved call (#37199)

* Fix leaked readClient in onFabricRemoved

When ReadClient::Close is called from onFabricRemove in InteractionModel Engine, readClient is destoryed and becomes not valid so that readClient->GetNextClient() will be use-after-free.

* Restyled by clang-format

* address comments

* Update TestRead.cpp

Co-authored-by: Boris Zbarsky <[email protected]>

* Clean up namespace in TestRead

--Remove unnecessary chip::Test and chip::app and app namespace

Restyled by clang-format

* address comments

---------

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>

Author: 3 people

src/app/InteractionModelEngine.cpp

@@ -1967,12 +1967,20 @@ void InteractionModelEngine::OnFabricRemoved(const FabricTable & fabricTable, Fa
     });
 
 #if CHIP_CONFIG_ENABLE_READ_CLIENT
-    for (auto * readClient = mpActiveReadClientList; readClient != nullptr; readClient = readClient->GetNextClient())
+    for (auto * readClient = mpActiveReadClientList; readClient != nullptr;)
     {
+        // ReadClient::Close may delete the read client so that readClient->GetNextClient() will be use-after-free.
+        // We need save readClient as nextReadClient before closing.
         if (readClient->GetFabricIndex() == fabricIndex)
         {
             ChipLogProgress(InteractionModel, "Fabric removed, deleting obsolete read client with FabricIndex: %u", fabricIndex);
+            auto * nextReadClient = readClient->GetNextClient();
             readClient->Close(CHIP_ERROR_IM_FABRIC_DELETED, false);
+            readClient = nextReadClient;
+        }
+        else
+        {
+            readClient = readClient->GetNextClient();
         }
     }
 #endif // CHIP_CONFIG_ENABLE_READ_CLIENT

src/app/InteractionModelEngine.h

@@ -324,7 +324,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
     /**
      * @brief Function decrements the number of subscriptions to resume counter - mNumOfSubscriptionsToResume.
      *        This should be called after we have completed a re-subscribe attempt on a persisted subscription wether the attempt
-     *        was succesful or not.
+     *        was successful or not.
      */
     void DecrementNumSubscriptionsToResume();
 #endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
@@ -714,7 +714,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
 #endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 #endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
 
-    FabricTable * mpFabricTable;
+    FabricTable * mpFabricTable = nullptr;
 
     CASESessionManager * mpCASESessionMgr = nullptr;