Bug 1452090: Only enable handle verifier on 32-bit Nightly and debug builds. r=jimm
authorBob Owen <bobowencode@gmail.com>
Mon, 09 Apr 2018 19:22:28 +0100
changeset 412407 d0c0f90a710d25141cb46dcbd2d6ed0c6832aa0b
parent 412406 b9cecc20bead9ab17dadfab185df01abe92a4db3
child 412408 43c585beaad3991ab02f3f93524a42326aec8eb4
push id101918
push userbobowencode@gmail.com
push dateMon, 09 Apr 2018 18:22:50 +0000
treeherdermozilla-inbound@d0c0f90a710d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1452090
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1452090: Only enable handle verifier on 32-bit Nightly and debug builds. r=jimm 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") &&