Bug 1671316 - Part2. Pass CrossExecTransferManager to FuncHookCrossProcess and Kernel32ExportsSolver. r=mhowell

The latest launcher process showed one of the top failures was `WriteProcessMemory` in
`CopyStubToChildProcess` failed with `ERROR_INVALID_ADDRESS` or `ERROR_NOACCESS`, that
is to store a trampoline address to the global variable of firefox.exe failed.  Its root
cause should be the same as bug 1662560, the executable was loaded onto a different
address from the browser process.

The fix is to to expand the usage of `CrossExecTransferManager` to `FuncHookCrossProcess`
and `Kernel32ExportsSolver`.

Depends on D94652

Differential Revision: https://phabricator.services.mozilla.com/D94653

browser/app/winlauncher/DllBlocklistInit.cpp

@@ -43,23 +43,28 @@ LauncherVoidResultWithLineInfo InitializeDllBlocklistOOPFromLauncher(
 static LauncherVoidResultWithLineInfo InitializeDllBlocklistOOPInternal(
     const wchar_t* aFullImagePath, HANDLE aChildProcess,
     const IMAGE_THUNK_DATA* aCachedNtdllThunk) {
+  nt::CrossExecTransferManager transferMgr(aChildProcess);
+  if (!transferMgr) {
+    return LAUNCHER_ERROR_FROM_WIN32(ERROR_BAD_EXE_FORMAT);
+  }
+
   freestanding::gK32.Init();
   if (freestanding::gK32.IsInitialized()) {
-    freestanding::gK32.Transfer(aChildProcess, &freestanding::gK32);
+    Unused << freestanding::gK32.Transfer(transferMgr, &freestanding::gK32);
   }
 
   CrossProcessDllInterceptor intcpt(aChildProcess);
   intcpt.Init(L"ntdll.dll");
 
   bool ok = freestanding::stub_NtMapViewOfSection.SetDetour(
-      aChildProcess, intcpt, "NtMapViewOfSection",
+      transferMgr, intcpt, "NtMapViewOfSection",
       &freestanding::patched_NtMapViewOfSection);
   if (!ok) {
     return LAUNCHER_ERROR_FROM_DETOUR_ERROR(intcpt.GetLastDetourError());
   }
 
   ok = freestanding::stub_LdrLoadDll.SetDetour(
-      aChildProcess, intcpt, "LdrLoadDll", &freestanding::patched_LdrLoadDll);
+      transferMgr, intcpt, "LdrLoadDll", &freestanding::patched_LdrLoadDll);
   if (!ok) {
     return LAUNCHER_ERROR_FROM_DETOUR_ERROR(intcpt.GetLastDetourError());
   }
@@ -75,10 +80,6 @@ static LauncherVoidResultWithLineInfo InitializeDllBlocklistOOPInternal(
   // it onto the child process's IAT, thus enabling the child process's hook to
   // safely make its ntdll calls.
 
-  nt::CrossExecTransferManager transferMgr(aChildProcess);
-  if (!transferMgr) {
-    return LAUNCHER_ERROR_FROM_WIN32(ERROR_BAD_EXE_FORMAT);
-  }
   const nt::PEHeaders& ourExeImage = transferMgr.LocalPEHeaders();
 
   // As part of our mitigation of binary tampering, copy our import directory

browser/app/winlauncher/freestanding/FunctionTableResolver.cpp

@@ -95,18 +95,18 @@ void Kernel32ExportsSolver::Resolve(RTL_RUN_ONCE& aRunOnce) {
   ::RtlRunOnceExecuteOnce(&aRunOnce, &ResolveOnce, this, nullptr);
 }
 
-void Kernel32ExportsSolver::Transfer(
-    HANDLE aTargetProcess, Kernel32ExportsSolver* aTargetAddress) const {
-  SIZE_T bytesWritten = 0;
-  BOOL ok = ::WriteProcessMemory(aTargetProcess, &aTargetAddress->mOffsets,
-                                 &mOffsets, sizeof(mOffsets), &bytesWritten);
-  if (!ok) {
-    return;
+LauncherVoidResult Kernel32ExportsSolver::Transfer(
+    nt::CrossExecTransferManager& aTransferMgr,
+    Kernel32ExportsSolver* aTargetAddress) const {
+  LauncherVoidResult writeResult = aTransferMgr.Transfer(
+      &aTargetAddress->mOffsets, &mOffsets, sizeof(mOffsets));
+  if (writeResult.isErr()) {
+    return writeResult.propagateErr();
   }
 
   State stateInChild = State::Initialized;
-  ::WriteProcessMemory(aTargetProcess, &aTargetAddress->mState, &stateInChild,
-                       sizeof(stateInChild), &bytesWritten);
+  return aTransferMgr.Transfer(&aTargetAddress->mState, &stateInChild,
+                               sizeof(stateInChild));
 }
 
 }  // namespace freestanding

browser/app/winlauncher/freestanding/FunctionTableResolver.h

@@ -48,8 +48,8 @@ class MOZ_TRIVIAL_CTOR_DTOR Kernel32ExportsSolver final
 
   void Init();
   void Resolve(RTL_RUN_ONCE& aRunOnce);
-  void Transfer(HANDLE aTargetProcess,
-                Kernel32ExportsSolver* aTargetAddress) const;
+  LauncherVoidResult Transfer(nt::CrossExecTransferManager& aTransferMgr,
+                              Kernel32ExportsSolver* aTargetAddress) const;
 };
 
 extern Kernel32ExportsSolver gK32;

browser/app/winlauncher/test/TestCrossProcessWin.cpp

@@ -6,6 +6,7 @@
 
 #define MOZ_USE_LAUNCHER_ERROR
 
+#include "freestanding/FunctionTableResolver.cpp"
 #include "mozilla/CmdLineAndEnvUtils.h"
 #include "mozilla/NativeNt.h"
 
@@ -21,6 +22,17 @@ void PrintLauncherError(const LauncherResult<T>& aResult,
          err.mError.AsHResult(), err.mFile, err.mLine);
 }
 
+#define VERIFY_FUNCTION_RESOLVED(mod, name)                      \
+  do {                                                           \
+    if (reinterpret_cast<FARPROC>(freestanding::gK32.m##name) != \
+        ::GetProcAddress(mod, #name)) {                          \
+      printf(                                                    \
+          "TEST-FAILED | TestCrossProcessWin | "                 \
+          "Kernel32ExportsSolver::" #name " did not match.\n");  \
+      return 1;                                                  \
+    }                                                            \
+  } while (0)
+
 class ChildProcess final {
   nsAutoHandle mChildProcess;
   nsAutoHandle mChildMainThread;
@@ -57,6 +69,19 @@ class ChildProcess final {
       return 1;
     }
 
+    static RTL_RUN_ONCE sRunOnce = RTL_RUN_ONCE_INIT;
+    freestanding::gK32.Resolve(sRunOnce);
+    if (!freestanding::gK32.IsResolved()) {
+      printf(
+          "TEST-FAILED | TestCrossProcessWin | "
+          "Kernel32ExportsSolver::Resolve failed.\n");
+    }
+
+    HMODULE k32mod = ::GetModuleHandleW(L"kernel32.dll");
+    VERIFY_FUNCTION_RESOLVED(k32mod, FlushInstructionCache);
+    VERIFY_FUNCTION_RESOLVED(k32mod, GetSystemInfo);
+    VERIFY_FUNCTION_RESOLVED(k32mod, VirtualProtect);
+
     return 0;
   }
 
@@ -150,7 +175,22 @@ int wmain(int argc, wchar_t* argv[]) {
     return 1;
   }
 
+  freestanding::gK32.Init();
+  if (!freestanding::gK32.IsInitialized()) {
+    printf(
+        "TEST-FAILED | TestCrossProcessWin | "
+        "Kernel32ExportsSolver initialization failed.\n");
+    return 1;
+  }
+
   LauncherVoidResult writeResult =
+      freestanding::gK32.Transfer(transferMgr, &freestanding::gK32);
+  if (writeResult.isErr()) {
+    PrintLauncherError(writeResult, "Kernel32ExportsSolver::Transfer failed");
+    return 1;
+  }
+
+  writeResult =
       transferMgr.Transfer(&ChildProcess::sExecutableImageBase,
                            &remoteImageBase.inspect(), sizeof(HMODULE));
   if (writeResult.isErr()) {

mozglue/misc/nsWindowsDllInterceptor.h

@@ -221,23 +221,19 @@ class MOZ_ONLY_USED_TO_AVOID_STATIC_CONSTRUCTORS FuncHookCrossProcess final {
   FuncHookCrossProcess() {}
 #endif  // defined(DEBUG)
 
-  bool Set(HANDLE aProcess, InterceptorT& aInterceptor, const char* aName,
-           FuncPtrT aHookDest) {
+  bool Set(nt::CrossExecTransferManager& aTransferMgr,
+           InterceptorT& aInterceptor, const char* aName, FuncPtrT aHookDest) {
     FuncPtrT origFunc;
     if (!aInterceptor.AddHook(aName, reinterpret_cast<intptr_t>(aHookDest),
                               reinterpret_cast<void**>(&origFunc))) {
       return false;
     }
 
-    bool ret = CopyStubToChildProcess(origFunc, aProcess);
-    if (!ret) {
-      aInterceptor.SetLastDetourError(FUNCHOOKCROSSPROCESS_COPYSTUB_ERROR,
-                                      ::GetLastError());
-    }
-    return ret;
+    return CopyStubToChildProcess(aTransferMgr, aInterceptor, origFunc);
   }
 
-  bool SetDetour(HANDLE aProcess, InterceptorT& aInterceptor, const char* aName,
+  bool SetDetour(nt::CrossExecTransferManager& aTransferMgr,
+                 InterceptorT& aInterceptor, const char* aName,
                  FuncPtrT aHookDest) {
     FuncPtrT origFunc;
     if (!aInterceptor.AddDetour(aName, reinterpret_cast<intptr_t>(aHookDest),
@@ -245,12 +241,7 @@ class MOZ_ONLY_USED_TO_AVOID_STATIC_CONSTRUCTORS FuncHookCrossProcess final {
       return false;
     }
 
-    bool ret = CopyStubToChildProcess(origFunc, aProcess);
-    if (!ret) {
-      aInterceptor.SetLastDetourError(FUNCHOOKCROSSPROCESS_COPYSTUB_ERROR,
-                                      ::GetLastError());
-    }
-    return ret;
+    return CopyStubToChildProcess(aTransferMgr, aInterceptor, origFunc);
   }
 
   explicit operator bool() const { return !!mOrigFunc; }
@@ -271,11 +262,21 @@ class MOZ_ONLY_USED_TO_AVOID_STATIC_CONSTRUCTORS FuncHookCrossProcess final {
 #endif  // defined(DEBUG)
 
  private:
-  bool CopyStubToChildProcess(FuncPtrT aStub, HANDLE aProcess) {
-    SIZE_T bytesWritten;
-    return ::WriteProcessMemory(aProcess, &mOrigFunc, &aStub, sizeof(FuncPtrT),
-                                &bytesWritten) &&
-           bytesWritten == sizeof(FuncPtrT);
+  bool CopyStubToChildProcess(nt::CrossExecTransferManager& aTransferMgr,
+                              InterceptorT& aInterceptor, FuncPtrT aStub) {
+    LauncherVoidResult writeResult =
+        aTransferMgr.Transfer(&mOrigFunc, &aStub, sizeof(FuncPtrT));
+    if (writeResult.isErr()) {
+#ifdef MOZ_USE_LAUNCHER_ERROR
+      const mozilla::WindowsError& err = writeResult.inspectErr().mError;
+#else
+      const mozilla::WindowsError& err = writeResult.inspectErr();
+#endif
+      aInterceptor.SetLastDetourError(FUNCHOOKCROSSPROCESS_COPYSTUB_ERROR,
+                                      err.AsHResult());
+      return false;
+    }
+    return true;
   }
 
  private:

mozglue/tests/interceptor/TestDllInterceptorCrossProcess.cpp

@@ -80,10 +80,18 @@ int ParentMain(int argc, wchar_t* argv[]) {
     return 1;
   }
 
+  mozilla::nt::CrossExecTransferManager transferMgr(childProcess);
+  if (!transferMgr) {
+    printf(
+        "TEST-UNEXPECTED-FAIL | DllInterceptorCrossProcess | "
+        "CrossExecTransferManager instantiation failed.\n");
+    return 1;
+  }
+
   mozilla::CrossProcessDllInterceptor intcpt(childProcess.get());
   intcpt.Init("TestDllInterceptorCrossProcess.exe");
 
-  if (!gOrigReturnResult.Set(childProcess.get(), intcpt, "ReturnResult",
+  if (!gOrigReturnResult.Set(transferMgr, intcpt, "ReturnResult",
                              &ReturnResultHook)) {
     printf(
         "TEST-UNEXPECTED-FAIL | DllInterceptorCrossProcess | Failed to add "