Fix a use after free for Managed Data BP (dotnet/coreclr#21205)
authorChuck Ries <chuckr@microsoft.com>
Tue, 27 Nov 2018 06:06:34 +0000 (22:06 -0800)
committerAndrew Au <cshung@gmail.com>
Tue, 27 Nov 2018 06:06:34 +0000 (22:06 -0800)
ShimProxyCallback::DataBreakpoint::DataBreakpointEvent was holding onto a bare
BYTE* for the CONTEXT rather than copying the buffer and taking ownership. Due to
lifetime, this resulted in a use after free. Apparently in retail code we got lucky
and this worked enough of the time that we never noticed it.

Commit migrated from https://github.com/dotnet/coreclr/commit/870267fac0b16ac246d6ba01f49ba4c6acd2319c

src/coreclr/src/debug/di/shimcallback.cpp

index 84ee345..e987f9b 100644 (file)
@@ -1392,7 +1392,7 @@ HRESULT ShimProxyCallback::DataBreakpoint(ICorDebugProcess* pProcess, ICorDebugT
         // callbacks parameters. These are strong references
         RSExtSmartPtr<ICorDebugProcess> m_pProcess;
         RSExtSmartPtr<ICorDebugThread> m_pThread;
-        BYTE* m_pContext;
+        CONTEXT m_context;
         ULONG32 m_contextSize;
 
     public:
@@ -1402,13 +1402,15 @@ HRESULT ShimProxyCallback::DataBreakpoint(ICorDebugProcess* pProcess, ICorDebugT
         {
             this->m_pProcess.Assign(pProcess);
             this->m_pThread.Assign(pThread);
-            this->m_pContext = pContext;
-            this->m_contextSize = contextSize;
+
+            _ASSERTE(contextSize == sizeof(CONTEXT));
+            this->m_contextSize = min(contextSize, sizeof(CONTEXT));
+            memcpy(&(this->m_context), pContext, this->m_contextSize);
         }
 
         HRESULT Dispatch(DispatchArgs args)
         {
-            return args.GetCallback4()->DataBreakpoint(m_pProcess, m_pThread, m_pContext, m_contextSize);
+            return args.GetCallback4()->DataBreakpoint(m_pProcess, m_pThread, reinterpret_cast<BYTE*>(&m_context), m_contextSize);
         }
     }; // end class AfterGarbageCollectionEvent