Bug 1452090 - Only enable handle verifier on 32-bit Nightly and debug builds. r=jimm, a=RyanVM
authorBob Owen <bobowencode@gmail.com>
Mon, 09 Apr 2018 19:22:28 +0100
changeset 449418 e85d91afc9a3e47595a6031612a20b33fdd50483
parent 449417 5a41bf296a861f25c9846b22a4b600f00e26e647
child 449419 dc880f246c3ec0710811efeb614be76099ee45b5
push id60
push userryanvm@gmail.com
push dateWed, 30 May 2018 18:57:55 +0000
treeherdermozilla-esr60@dc880f246c3e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm, RyanVM
bugs1452090
milestone60.0.2
Bug 1452090 - Only enable handle verifier on 32-bit Nightly and debug builds. r=jimm, a=RyanVM This also adds the ability to enable it using the environement variable MOZ_ENABLE_HANDLE_VERIFIER.
security/sandbox/win/SandboxInitialization.cpp
toolkit/xre/test/win/TestDllInterceptor.cpp
--- a/security/sandbox/win/SandboxInitialization.cpp
+++ b/security/sandbox/win/SandboxInitialization.cpp
@@ -2,35 +2,127 @@
 /* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "SandboxInitialization.h"
 
 #include "base/memory/ref_counted.h"
+#include "nsWindowsDllInterceptor.h"
 #include "sandbox/win/src/sandbox_factory.h"
 #include "mozilla/sandboxing/permissionsService.h"
 
 namespace mozilla {
 namespace sandboxing {
 
+typedef BOOL(WINAPI* CloseHandle_func) (HANDLE hObject);
+static CloseHandle_func stub_CloseHandle = nullptr;
+
+typedef BOOL(WINAPI* DuplicateHandle_func)(HANDLE hSourceProcessHandle,
+                                           HANDLE hSourceHandle,
+                                           HANDLE hTargetProcessHandle,
+                                           LPHANDLE lpTargetHandle,
+                                           DWORD dwDesiredAccess,
+                                           BOOL bInheritHandle,
+                                           DWORD dwOptions);
+static DuplicateHandle_func stub_DuplicateHandle = nullptr;
+
+static BOOL WINAPI
+patched_CloseHandle(HANDLE hObject)
+{
+  // Check all handles being closed against the sandbox's tracked handles.
+  base::win::OnHandleBeingClosed(hObject);
+  return stub_CloseHandle(hObject);
+}
+
+static BOOL WINAPI
+patched_DuplicateHandle(HANDLE hSourceProcessHandle,
+                        HANDLE hSourceHandle,
+                        HANDLE hTargetProcessHandle,
+                        LPHANDLE lpTargetHandle,
+                        DWORD dwDesiredAccess,
+                        BOOL bInheritHandle,
+                        DWORD dwOptions)
+{
+  // If closing a source handle from our process check it against the sandbox's
+  // tracked handles.
+  if ((dwOptions & DUPLICATE_CLOSE_SOURCE) &&
+    (GetProcessId(hSourceProcessHandle) == ::GetCurrentProcessId())) {
+    base::win::OnHandleBeingClosed(hSourceHandle);
+  }
+
+  return stub_DuplicateHandle(hSourceProcessHandle, hSourceHandle,
+                              hTargetProcessHandle, lpTargetHandle,
+                              dwDesiredAccess, bInheritHandle, dwOptions);
+}
+
+static WindowsDllInterceptor Kernel32Intercept;
+
+static bool
+EnableHandleCloseMonitoring()
+{
+  Kernel32Intercept.Init("kernel32.dll");
+  bool hooked =
+    Kernel32Intercept.AddHook("CloseHandle",
+                              reinterpret_cast<intptr_t>(patched_CloseHandle),
+                              (void**)&stub_CloseHandle);
+  if (!hooked) {
+    return false;
+  }
+
+  hooked =
+    Kernel32Intercept.AddHook("DuplicateHandle",
+                              reinterpret_cast<intptr_t>(patched_DuplicateHandle),
+                              (void**)&stub_DuplicateHandle);
+  if (!hooked) {
+    return false;
+  }
+
+  return true;
+}
+
+static bool
+ShouldDisableHandleVerifier()
+{
+#if defined(_X86_) && (defined(NIGHTLY_BUILD) || defined(DEBUG))
+  // Chromium only has the verifier enabled for 32-bit and our close monitoring
+  // hooks cause debug assertions for 64-bit anyway.
+  // For x86 keep the verifier enabled by default only for Nightly or debug.
+  return false;
+#else
+  return !getenv("MOZ_ENABLE_HANDLE_VERIFIER");
+#endif
+}
+
+static void
+InitializeHandleVerifier()
+{
+  // Disable the handle verifier if we don't want it or can't enable the close
+  // monitoring hooks.
+  if (ShouldDisableHandleVerifier() || !EnableHandleCloseMonitoring()) {
+    base::win::DisableHandleVerifier();
+  }
+}
+
 static sandbox::TargetServices*
 InitializeTargetServices()
 {
   sandbox::TargetServices* targetServices =
     sandbox::SandboxFactory::GetTargetServices();
   if (!targetServices) {
     return nullptr;
   }
 
   if (targetServices->Init() != sandbox::SBOX_ALL_OK) {
     return nullptr;
   }
 
+  InitializeHandleVerifier();
+
   return targetServices;
 }
 
 sandbox::TargetServices*
 GetInitializedTargetServices()
 {
   static sandbox::TargetServices* sInitializedTargetServices =
     InitializeTargetServices();
@@ -61,16 +153,18 @@ InitializeBrokerServices()
   // Precreate the desktop and window station used by the renderers.
   // IMPORTANT: This piece of code needs to run as early as possible in the
   // process because it will initialize the sandbox broker, which requires
   // the process to swap its window station. During this time all the UI
   // will be broken. This has to run before threads and windows are created.
   scoped_refptr<sandbox::TargetPolicy> policy = brokerServices->CreatePolicy();
   sandbox::ResultCode result = policy->CreateAlternateDesktop(true);
 
+  InitializeHandleVerifier();
+
   return brokerServices;
 }
 
 sandbox::BrokerServices*
 GetInitializedBrokerServices()
 {
   static sandbox::BrokerServices* sInitializedBrokerServices =
     InitializeBrokerServices();
--- a/toolkit/xre/test/win/TestDllInterceptor.cpp
+++ b/toolkit/xre/test/win/TestDllInterceptor.cpp
@@ -431,16 +431,30 @@ bool TestTlsAlloc(void* aFunc)
 
 bool TestTlsFree(void* aFunc)
 {
   auto patchedTlsFree =
     reinterpret_cast<decltype(&TlsFree)>(aFunc);
   return sTlsIndex != 0 && patchedTlsFree(sTlsIndex);
 }
 
+bool TestCloseHandle(void* aFunc)
+{
+  auto patchedCloseHandle =
+    reinterpret_cast<decltype(&CloseHandle)>(aFunc);
+  return patchedCloseHandle(0) == FALSE;
+}
+
+bool TestDuplicateHandle(void* aFunc)
+{
+  auto patchedDuplicateHandle =
+    reinterpret_cast<decltype(&DuplicateHandle)>(aFunc);
+  return patchedDuplicateHandle(0, 0, 0, 0, 0, 0, 0) == FALSE;
+}
+
 bool TestPrintDlgW(void* aFunc)
 {
   auto patchedPrintDlgW =
     reinterpret_cast<decltype(&PrintDlgW)>(aFunc);
   patchedPrintDlgW(0);
   return true;
 }
 
@@ -689,16 +703,18 @@ int main()
 #endif
       MaybeTestHook(ShouldTestTipTsf(), TestProcessCaretEvents, "tiptsf.dll", "ProcessCaretEvents") &&
 #ifdef _M_IX86
       TestHook(TestSendMessageTimeoutW, "user32.dll", "SendMessageTimeoutW") &&
 #endif
       TestHook(TestSetCursorPos, "user32.dll", "SetCursorPos") &&
       TestHook(TestTlsAlloc, "kernel32.dll", "TlsAlloc") &&
       TestHook(TestTlsFree, "kernel32.dll", "TlsFree") &&
+      TestHook(TestCloseHandle, "kernel32.dll", "CloseHandle") &&
+      TestHook(TestDuplicateHandle, "kernel32.dll", "DuplicateHandle") &&
 
       TestHook(TestInternetOpenA, "wininet.dll", "InternetOpenA") &&
       TestHook(TestInternetCloseHandle, "wininet.dll", "InternetCloseHandle") &&
       TestHook(TestInternetConnectA, "wininet.dll", "InternetConnectA") &&
       TestHook(TestInternetQueryDataAvailable, "wininet.dll", "InternetQueryDataAvailable") &&
       TestHook(TestInternetReadFile, "wininet.dll", "InternetReadFile") &&
       TestHook(TestInternetWriteFile, "wininet.dll", "InternetWriteFile") &&
       TestHook(TestInternetSetOptionA, "wininet.dll", "InternetSetOptionA") &&