Changeset 78088 in vbox

Timestamp:

Apr 10, 2019 1:36:19 PM (6 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

129944

Message:

Main/glue: Consolidated the bool parameters of com::Initialize() into a bit mask; added COM hack to prevent out-of-process IRundown::DoCallback calls. bugref:9425

Location:

trunk

Files:

• include/VBox/com/com.h (modified) (3 diffs)
• src/VBox/Frontends/VBoxBugReport/VBoxBugReport.cpp (modified) (1 diff)
• src/VBox/Frontends/VirtualBox/src/globals/COMDefs.cpp (modified) (1 diff)
• src/VBox/Main/cbinding/VBoxCAPI.cpp (modified) (3 diffs)
• src/VBox/Main/glue/initterm.cpp (modified) (9 diffs)
• src/VBox/Main/src-server/win/svcmain.cpp (modified) (2 diffs)

trunk/include/VBox/com/com.h

@@ -39 +39 @@
 {
 
+/** @name VBOX_COM_INIT_F_XXX - flags for com::Initialize().
+ * @{ */
+/** Windows: Caller is the GUI and needs a STA rather than MTA apartment. */
+#define VBOX_COM_INIT_F_GUI             RT_BIT_32(0)
+/** Windows: Auto registration updating, if privileged enough. */
+#define VBOX_COM_INIT_F_AUTO_REG_UPDATE RT_BIT_32(1)
+/** Windows: Opt-out of COM patching (client code should do this). */
+#define VBOX_COM_INIT_F_NO_COM_PATCHING RT_BIT_32(2)
+/** The default flags. */
+#define VBOX_COM_INIT_F_DEFAULT         (VBOX_COM_INIT_F_AUTO_REG_UPDATE)
+/** @} */
+
 /**
  *  Initializes the COM runtime.
@@ -46 +58 @@
  *  @param fGui             if call is performed on the GUI thread
  *  @param fAutoRegUpdate   if to do auto MS COM registration updates.
+ *  @param fNoComPatching   Set this to skip the COM patching.
  *  @return COM result code
  */
-HRESULT Initialize(bool fGui = false, bool fAutoRegUpdate = true);
+HRESULT Initialize(uint32_t fInitFlags = VBOX_COM_INIT_F_DEFAULT);
 
 /**
@@ -114 +127 @@
                      PRTERRINFO pErrInfo);
 
+#ifdef RT_OS_WINDOWS
+void PatchComBugs(void);
+#endif
+
 } /* namespace com */
 

trunk/src/VBox/Frontends/VBoxBugReport/VBoxBugReport.cpp

@@ -737 +737 @@
             throw RTCError("Out of memory\n");
 
-        handleComError(com::Initialize(), "Failed to initialize COM");
+        handleComError(com::Initialize(VBOX_COM_INIT_F_DEFAULT | VBOX_COM_INIT_F_NO_COM_PATCHING), "Failed to initialize COM");
 
         MachineInfoList list;

trunk/src/VBox/Frontends/VirtualBox/src/globals/COMDefs.cpp

@@ -90 +90 @@
     LogFlowFuncEnter();
 
-    HRESULT rc = com::Initialize(fGui);
+    HRESULT rc = com::Initialize(fGui ? VBOX_COM_INIT_F_DEFAULT | VBOX_COM_INIT_F_GUI : VBOX_COM_INIT_F_DEFAULT);
 
 #if defined (VBOX_WITH_XPCOM)

trunk/src/VBox/Main/cbinding/VBoxCAPI.cpp

@@ -394 +394 @@
         sessionIID = IID_ISession;
 
-    HRESULT rc = com::Initialize();
+    HRESULT rc = com::Initialize(VBOX_COM_INIT_F_DEFAULT | VBOX_COM_INIT_F_NO_COM_PATCHING);
     if (FAILED(rc))
     {
@@ -669 +669 @@
         virtualBoxClientIID = IID_IVirtualBoxClient;
 
-    HRESULT rc = com::Initialize();
+    HRESULT rc = com::Initialize(VBOX_COM_INIT_F_DEFAULT | VBOX_COM_INIT_F_NO_COM_PATCHING);
     if (FAILED(rc))
     {
@@ -728 +728 @@
 VBoxClientThreadInitialize(void)
 {
-    return com::Initialize();
+    return com::Initialize(VBOX_COM_INIT_F_DEFAULT | VBOX_COM_INIT_F_NO_COM_PATCHING);
 }
 

trunk/src/VBox/Main/glue/initterm.cpp

@@ -19 +19 @@
 #if !defined(VBOX_WITH_XPCOM)
 
+# if !defined(_WIN32_WINNT) || _WIN32_WINNT < 0x600
+#  undef  _WIN32_WINNT
+#  define _WIN32_WINNT 0x600 /* GetModuleHandleExW */
+# endif
 # include <iprt/nt/nt-and-windows.h>
 # include <iprt/win/objbase.h>
+# include <iprt/win/rpcproxy.h>
+# include <rpcasync.h>
 
 #else /* !defined(VBOX_WITH_XPCOM) */
@@ -51 +57 @@
 #include <iprt/path.h>
 #include <iprt/string.h>
+#include <iprt/system.h>
 #include <iprt/thread.h>
 
@@ -192 +199 @@
 
 /**
+ * Replacement function for the InvokeStub method for the IRundown stub.
+ */
+static HRESULT STDMETHODCALLTYPE Rundown_InvokeStub(IRpcStubBuffer *pThis, RPCOLEMESSAGE *pMsg, IRpcChannelBuffer *pBuf)
+{
+    /*
+     * Our mission here is to prevent remote calls to methods #8 and #9,
+     * as these contain raw pointers to callback functions.
+     *
+     * Note! APIs like I_RpcServerInqTransportType, I_RpcBindingInqLocalClientPID
+     *       and RpcServerInqCallAttributesW are not usable in this context without
+     *       a rpc binding handle (latter two).
+     *
+     * P.S.  In more recent windows versions, the buffer implements a interface
+     *       IID_IRpcChannelBufferMarshalingContext (undocumented) which has a
+     *       GetIMarshallingContextAttribute() method that will return the client PID
+     *       when asking for attribute #0x8000000e.
+     */
+    uint32_t const iMethod = pMsg->iMethod & 0xffff; /* Uncertain, but there are hints that the upper bits are flags. */
+    HRESULT        hrc;
+    if (   (   iMethod != 8
+            && iMethod != 9)
+        || (pMsg->rpcFlags & RPCFLG_LOCAL_CALL) )
+        hrc = CStdStubBuffer_Invoke(pThis, pMsg, pBuf);
+    else
+    {
+        LogRel(("Rundown_InvokeStub: Rejected call to CRundown::%s: rpcFlags=%#x cbBuffer=%#x dataRepresentation=%d buffer=%p:{%.*Rhxs} reserved1=%p reserved2={%p,%p,%p,%p,%p}\n",
+                pMsg->iMethod == 8 ? "DoCallback" : "DoNonreentrantCallback", pMsg->rpcFlags, pMsg->cbBuffer,
+                pMsg->dataRepresentation, pMsg->Buffer, RT_VALID_PTR(pMsg->Buffer) ? pMsg->cbBuffer : 0, pMsg->Buffer,
+                pMsg->reserved1, pMsg->reserved2[0], pMsg->reserved2[1], pMsg->reserved2[2], pMsg->reserved2[3], pMsg->reserved2[4]));
+        hrc = E_ACCESSDENIED;
+    }
+    return hrc;
+}
+
+/**
+ * Replaces the IRundown InvokeStub method with Rundown_InvokeStub so we can
+ * reject remote calls to a couple of misdesigned methods.
+ */
+void PatchComBugs(void)
+{
+    static volatile bool s_fPatched = false;
+    if (s_fPatched)
+        return;
+
+    /*
+     * The combase.dll / ole32.dll is exporting a DllGetClassObject function
+     * that is implemented using NdrDllGetClassObject just like our own
+     * proxy/stub DLL.  This means we can get at the stub interface lists,
+     * since what NdrDllGetClassObject has CStdPSFactoryBuffer as layout.
+     *
+     * Note! Tried using CoRegisterPSClsid instead of this mess, but no luck.
+     */
+    /* Locate the COM DLL, it must be loaded by now: */
+    HMODULE hmod = GetModuleHandleW(L"COMBASE.DLL");
+    if (!hmod)
+        hmod = GetModuleHandleW(L"OLE32.DLL"); /* w7 */
+    AssertReturnVoid(hmod != NULL);
+
+    /* Resolve the class getter: */
+    LPFNGETCLASSOBJECT pfnGetClassObject = (LPFNGETCLASSOBJECT)GetProcAddress(hmod, "DllGetClassObject");
+    AssertReturnVoid(pfnGetClassObject != NULL);
+
+    /* Get the factory instance: */
+    static const CLSID   s_PSOlePrx32ClsId = {0x00000320,0x0000,0x0000,{0xc0,0x00,0x00,0x00,0x00,0x00,0x00,0x46}};
+    CStdPSFactoryBuffer *pFactoryBuffer = NULL;
+    HRESULT hrc = pfnGetClassObject(s_PSOlePrx32ClsId, IID_IPSFactoryBuffer, (void **)&pFactoryBuffer);
+    AssertMsgReturnVoid(SUCCEEDED(hrc),  ("hrc=%Rhrc\n",  hrc));
+    AssertReturnVoid(pFactoryBuffer != NULL);
+
+    /*
+     * Search thru the file list for the interface we want to patch.
+     */
+    static const IID s_IID_Rundown = {0x00000134,0x0000,0x0000,{0xc0,0x00,0x00,0x00,0x00,0x00,0x00,0x46}};
+    decltype(CStdStubBuffer_Invoke) *pfnInvoke = (decltype(pfnInvoke))GetProcAddress(hmod, "CStdStubBuffer_Invoke");
+    if (!pfnInvoke)
+        pfnInvoke = (decltype(pfnInvoke))GetProcAddress(GetModuleHandleW(L"RPCRT4.DLL"), "CStdStubBuffer_Invoke");
+
+    unsigned cPatched = 0;
+    unsigned cAlreadyPatched = 0;
+    Assert(pFactoryBuffer->pProxyFileList != NULL);
+    for (ProxyFileInfo const **ppCur = pFactoryBuffer->pProxyFileList; *ppCur != NULL; ppCur++)
+    {
+        ProxyFileInfo const *pCur = *ppCur;
+
+        if (pCur->pStubVtblList)
+        {
+            for (PCInterfaceStubVtblList const *ppCurStub = pCur->pStubVtblList; *ppCurStub != NULL; ppCurStub++)
+            {
+                PCInterfaceStubVtblList const pCurStub = *ppCurStub;
+                IID const *piid = pCurStub->header.piid;
+                if (piid)
+                {
+                    if (IsEqualIID(*piid, s_IID_Rundown))
+                    {
+                        if (pCurStub->Vtbl.Invoke == pfnInvoke)
+                        {
+                            DWORD fOld = 0;
+                            if (VirtualProtect(&pCurStub->Vtbl.Invoke, sizeof(pCurStub->Vtbl.Invoke), PAGE_READWRITE, &fOld))
+                            {
+                                pCurStub->Vtbl.Invoke = Rundown_InvokeStub;
+                                VirtualProtect(&pCurStub->Vtbl.Invoke, sizeof(pCurStub->Vtbl.Invoke), fOld, &fOld);
+                                cPatched++;
+                            }
+                            else
+                                AssertMsgFailed(("%d\n", GetLastError()));
+                        }
+                        else
+                            cAlreadyPatched++;
+                    }
+                }
+            }
+       }
+    }
+
+    /* done */
+    pFactoryBuffer->lpVtbl->Release((IPSFactoryBuffer *)pFactoryBuffer);
+
+    /*
+     * If we patched anything we should try prevent being unloaded.
+     */
+    if (cPatched > 0)
+    {
+        s_fPatched = true;
+        HMODULE hmodSelf;
+        AssertLogRelMsg(GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_PIN,
+                                           (LPCWSTR)(uintptr_t)Rundown_InvokeStub, &hmodSelf),
+                        ("last error: %u; Rundown_InvokeStub=%p\n", GetLastError(), Rundown_InvokeStub));
+    }
+    else
+        AssertLogRelMsg(cAlreadyPatched > 0, ("COM patching of IRundown failed!\n"));
+}
+
+
+/**
  *  The COM main thread handle. (The first caller of com::Initialize().)
  */
@@ -239 +380 @@
  * @return S_OK on success and a COM result code in case of failure.
  */
-HRESULT Initialize(bool fGui /*= false*/, bool fAutoRegUpdate /*= true*/)
+HRESULT Initialize(uint32_t fInitFlags /*=VBOX_COM_INIT_F_DEFAULT*/)
 {
     HRESULT rc = E_FAIL;
-    NOREF(fAutoRegUpdate);
 
 #if !defined(VBOX_WITH_XPCOM)
@@ -252 +392 @@
      * API users already active, as that could lead to a bit of a mess.
      */
-    if (   fAutoRegUpdate
+    if (   (fInitFlags & VBOX_COM_INIT_F_AUTO_REG_UPDATE)
         && gCOMMainThread == NIL_RTTHREAD)
     {
@@ -303 +443 @@
      * !!!!! Please think twice before touching this code !!!!!
      */
-    DWORD flags = fGui ?
-                  COINIT_APARTMENTTHREADED
-                | COINIT_SPEED_OVER_MEMORY
-                :
-                  COINIT_MULTITHREADED
-                | COINIT_DISABLE_OLE1DDE
-                | COINIT_SPEED_OVER_MEMORY;
+    DWORD flags = fInitFlags & VBOX_COM_INIT_F_GUI
+                ?   COINIT_APARTMENTTHREADED
+                  | COINIT_SPEED_OVER_MEMORY
+                :   COINIT_MULTITHREADED
+                  | COINIT_DISABLE_OLE1DDE
+                  | COINIT_SPEED_OVER_MEMORY;
 
     rc = CoInitializeEx(NULL, flags);
@@ -316 +455 @@
      * "already initialized using the same apartment model") */
     AssertMsg(rc == S_OK || rc == S_FALSE, ("rc=%08X\n", rc));
+
+    /*
+     * IRundown has unsafe two methods we need to patch to prevent remote access.
+     * Do that before we start using COM and open ourselves to possible attacks.
+     */
+    if (!(fInitFlags & VBOX_COM_INIT_F_NO_COM_PATCHING))
+        PatchComBugs();
 
     /* To be flow compatible with the XPCOM case, we return here if this isn't
@@ -328 +474 @@
         fRc = false;
 
-    if (fGui)
+    if (fInitFlags & VBOX_COM_INIT_F_GUI)
         Assert(RTThreadIsMain(hSelf));
 
@@ -350 +496 @@
 
     /* Unused here */
-    NOREF(fGui);
+    RT_NOREF(fInitFlags);
 
     if (ASMAtomicXchgBool(&gIsXPCOMInitialized, true) == true)

trunk/src/VBox/Main/src-server/win/svcmain.cpp

@@ -945 +945 @@
                 "BU", "IL", "DI", "D", RTBldCfgVersion(), RTBldCfgRevision(), "BU", "IL", "DI", "D");
 
-    int nRet = 0;
-    HRESULT hRes = com::Initialize(false /*fGui*/, fRun /*fAutoRegUpdate*/);
+    AssertCompile(VBOX_COM_INIT_F_DEFAULT == VBOX_COM_INIT_F_AUTO_REG_UPDATE);
+    HRESULT hRes = com::Initialize(fRun ? VBOX_COM_INIT_F_AUTO_REG_UPDATE : 0);
     AssertLogRelMsg(SUCCEEDED(hRes), ("SVCMAIN: init failed: %Rhrc\n", hRes));
 
@@ -955 +955 @@
     g_pModule->dwThreadID = GetCurrentThreadId();
 
+    int nRet = 0;
     if (!fRun)
     {