From 95964425e6928b6bc4e934445c34b3f89846e6a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= <mamoreau@devolutions.net>
Date: Thu, 24 Sep 2026 14:38:42 -0400
Subject: [PATCH] Fix mstscax control instance and TS property-set reference
 leaks

- CClassFactory::CreateInstance held onto the reference IClassFactory::
  CreateInstance returns for the real mstscax control after wrapping it
  in CMsRdpClient, which already takes its own AddRef in its
  constructor. The factory never released its copy, so the real
  control's refcount never reached zero when the wrapper was
  destroyed. This is the primary reported leak: mstscax!CSND::SND_Main
  and mstscax!CRCV::RCVMain threads (and ~100 handles) parked forever
  per closed embedded RDP session.

- CMsRdpClient destructor released its wrapper objects
  (m_pMsRdpExtendedSettings, m_pMsRdpExInstance) after releasing the
  underlying control interfaces, even though those wrappers hold
  references into control-internal objects owned by the control.
  Reordered to release wrappers first.

- CMsRdpPropertySet stored the internal ITSPropertySet/TS core-props
  IUnknown it was given without taking its own reference, but its
  destructor unconditionally released it anyway, decrementing a
  refcount it never incremented. This could prematurely free
  control-internal property-set objects still in use elsewhere,
  producing heap corruption during teardown. Added the missing AddRef
  to balance the destructor Release.

Verified: full x64 build, ctest 31/31 passing.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 dll/MsRdpClient.cpp | 36 ++++++++++++++++++++++--------------
 dll/RdpSettings.cpp |  1 +
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/dll/MsRdpClient.cpp b/dll/MsRdpClient.cpp
index a0d1ec3..c13ada8 100644
--- a/dll/MsRdpClient.cpp
+++ b/dll/MsRdpClient.cpp
@@ -190,6 +190,22 @@ class CMsRdpClient : public IMsRdpClient10
         MsRdpEx_D3D11Capture_ReleaseInstance(
             (IMsRdpExInstance*)m_pMsRdpExInstance);
 
+        // Release our wrapper objects before the control's interfaces:
+        // they hold references to control-internal objects (TS property
+        // sets) whose lifetime is owned by the wrapped control.
+        if (m_pMsRdpExtendedSettings) {
+            IMsRdpExtendedSettings* pMsRdpExtendedSettings = (IMsRdpExtendedSettings*) m_pMsRdpExtendedSettings;
+            pMsRdpExtendedSettings->Release();
+            pMsRdpExtendedSettings = NULL;
+        }
+
+        if (m_pMsRdpExInstance) {
+            if (m_instanceRegistered)
+                MsRdpEx_InstanceManager_Remove(m_pMsRdpExInstance);
+            ((IMsRdpExInstance*)m_pMsRdpExInstance)->Release();
+            m_pMsRdpExInstance = NULL;
+        }
+
         m_pUnknown->Release();
         if (m_pDispatch) m_pDispatch->Release();
         if (m_pMsTscAx) m_pMsTscAx->Release();
@@ -203,19 +219,6 @@ class CMsRdpClient : public IMsRdpClient10
         if (m_pMsRdpClient8) m_pMsRdpClient8->Release();
         if (m_pMsRdpClient9) m_pMsRdpClient9->Release();
         if (m_pMsRdpClient10) m_pMsRdpClient10->Release();
-
-        if (m_pMsRdpExtendedSettings) {
-            IMsRdpExtendedSettings* pMsRdpExtendedSettings = (IMsRdpExtendedSettings*) m_pMsRdpExtendedSettings;
-            pMsRdpExtendedSettings->Release();
-            pMsRdpExtendedSettings = NULL;
-        }
-        
-        if (m_pMsRdpExInstance) {
-            if (m_instanceRegistered)
-                MsRdpEx_InstanceManager_Remove(m_pMsRdpExInstance);
-            ((IMsRdpExInstance*)m_pMsRdpExInstance)->Release();
-            m_pMsRdpExInstance = NULL;
-        }
     }
 
     // IUnknown interface
@@ -885,9 +888,14 @@ class CClassFactory : IClassFactory
                 (m_clsid == CLSID_MsRdpClient10NotSafeForScripting) ||
                 (m_clsid == CLSID_MsRdpClient11NotSafeForScripting))
             {
-                CMsRdpClient* pMsRdpClient = new CMsRdpClient((IUnknown*)*ppvObject);
+                IUnknown* pUnknown = (IUnknown*)*ppvObject;
+                CMsRdpClient* pMsRdpClient = new CMsRdpClient(pUnknown);
                 hr = pMsRdpClient->QueryInterface(riid, ppvObject);
                 pMsRdpClient->Release();
+                // Release the class factory's reference on the wrapped control:
+                // the wrapper holds its own reference from its constructor, and
+                // keeping the factory's reference would leak the control forever.
+                pUnknown->Release();
             }
         }
 
diff --git a/dll/RdpSettings.cpp b/dll/RdpSettings.cpp
index a050534..5f5d790 100644
--- a/dll/RdpSettings.cpp
+++ b/dll/RdpSettings.cpp
@@ -463,6 +463,7 @@ class CMsRdpPropertySet : public IMsRdpExtendedSettings
     {
         m_refCount = 1;
         m_pUnknown = pUnknown;
+        pUnknown->AddRef(); // balances m_pUnknown->Release() in the destructor
         pUnknown->QueryInterface(IID_ITSPropertySet, (LPVOID*)&m_pTSPropertySet);
 
         if (m_pTSPropertySet)