Bug 1503538: Part 5 - Add static DLL dependency blocking to SandboxBroker; r=bobowen
authorAaron Klotz <aklotz@mozilla.com>
Fri, 12 Apr 2019 09:47:13 +0000
changeset 469441 ec1f3a922d56e0a9393415872ee1d30c2f895fd7
parent 469440 c82a828a6e4dc4677903384149ea2d10115f418e
child 469442 c2659af33baf37f9e982087e0982677dae33fe4b
child 469445 fa9332a8ea937098a1a6d79ed4bc9028e3917e62
push id112789
push usernbeleuzu@mozilla.com
push dateSun, 14 Apr 2019 21:52:28 +0000
treeherdermozilla-inbound@ec1f3a922d56 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1503538
milestone68.0a1
first release with
nightly linux32
ec1f3a922d56 / 68.0a1 / 20190414214746 / files
nightly linux64
ec1f3a922d56 / 68.0a1 / 20190414214746 / files
nightly mac
ec1f3a922d56 / 68.0a1 / 20190414214746 / files
nightly win32
ec1f3a922d56 / 68.0a1 / 20190414214746 / files
nightly win64
ec1f3a922d56 / 68.0a1 / 20190414214746 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1503538: Part 5 - Add static DLL dependency blocking to SandboxBroker; r=bobowen Depends on D27146 Differential Revision: https://phabricator.services.mozilla.com/D27147
security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -6,16 +6,17 @@
 
 #include "sandboxBroker.h"
 
 #include <string>
 
 #include "base/win/windows_version.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/ClearOnShutdown.h"
+#include "mozilla/ImportDir.h"
 #include "mozilla/Logging.h"
 #include "mozilla/NSPRLogModulesParser.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/WindowsVersion.h"
 #include "nsAppDirectoryServiceDefs.h"
 #include "nsCOMPtr.h"
@@ -268,16 +269,61 @@ bool SandboxBroker::LaunchApp(const wcha
 
     return false;
   } else if (sandbox::SBOX_ALL_OK != last_warning) {
     // If there was a warning (but the result was still ok), log it and proceed.
     LOG_W("Warning on SpawnTarget with last_error=%d, last_warning=%d",
           last_error, last_warning);
   }
 
+  // moduleHandle holds a strong reference to the module, whereas realBase
+  // is weak and might reference a module from another process (and thus must
+  // not be considered valid to pass in to any Win32 APIs from within this
+  // process).
+  nsModuleHandle moduleHandle;
+  HMODULE realBase = nullptr;
+  if (XRE_GetChildProcBinPathType(aProcessType) == BinPathType::Self) {
+    // We use GetModuleHandleEx here so that we increment the module's refcount
+    HMODULE ourExe;
+    if (::GetModuleHandleExW(0, nullptr, &ourExe)) {
+      moduleHandle.own(ourExe);
+      // We can assign ourExe to realBase because the child process's binary is
+      // the same as ours; ASLR will map it to the same address.
+      realBase = ourExe;
+    }
+  } else {
+    // Load the child executable as a datafile so that we can examine its
+    // headers without doing a full load with dependencies and such.
+    moduleHandle.own(::LoadLibraryExW(aPath, nullptr, LOAD_LIBRARY_AS_DATAFILE));
+    LauncherResult<HMODULE> procExeModule =
+      nt::GetProcessExeModule(targetInfo.hProcess);
+    if (procExeModule.isOk()) {
+      realBase = procExeModule.unwrap();
+    } else {
+      LOG_E("nt::GetProcessExeModule failed with HRESULT 0x%08lX",
+            procExeModule.unwrapErr().AsHResult());
+    }
+  }
+
+  if (moduleHandle && realBase) {
+    nt::PEHeaders exeImage(moduleHandle.get());
+    if (!!exeImage) {
+      LauncherVoidResult importsRestored =
+        RestoreImportDirectory(aPath, exeImage, targetInfo.hProcess, realBase);
+      if (importsRestored.isErr()) {
+        LOG_E("Failed to restore import directory with HRESULT 0x%08lX",
+              importsRestored.unwrapErr().AsHResult());
+        TerminateProcess(targetInfo.hProcess, 1);
+        CloseHandle(targetInfo.hThread);
+        CloseHandle(targetInfo.hProcess);
+        return false;
+      }
+    }
+  }
+
   // The sandboxed process is started in a suspended state, resume it now that
   // we've set things up.
   ResumeThread(targetInfo.hThread);
   CloseHandle(targetInfo.hThread);
 
   // Return the process handle to the caller
   *aProcessHandle = targetInfo.hProcess;