Bug 1811590 - always calculate whether a module is dependent r=yjuglaret

A few changes here:
- rename IsDependentModule() to IsInjectedDependentModule(), as that's closer to what it does (and confused me when I was reading this code)
- Move the `#if defined(EARLY_BETA_OR_EARLIER)` out to the calling function so that we always correctly calculate whether a module is dependent, and change it to NIGHTLY_BUILD re bug 1806041
- Since we're now always detecting whether a module is dependent, it seems like a good idea if we were going to be blocking it to make it NoOpEntryPoint even if we're not NIGHTLY_BUILD. This will help if a user adds such a DLL to the dynamic blocklist; I think if that were to happen right now Firefox would crash on launch?

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

browser/app/winlauncher/freestanding/DllBlocklist.cpp

@@ -143,10 +143,23 @@ NativeNtBlockSet_Write(CrashReporter::AnnotationWriter& aWriter) {
 }
 
 enum class BlockAction {
+  // Allow the DLL to be loaded.
   Allow,
+  // Substitute in a different DLL to be loaded instead of this one?
+  // This is intended to be used for Layered Service Providers, which
+  // cannot be blocked in the normal way. Note that this doesn't seem
+  // to be actually implemented right now, and no entries in the blocklist
+  // use it.
   SubstituteLSP,
+  // There was an error in determining whether we should block this DLL.
+  // It will be blocked.
   Error,
+  // Block the DLL from loading.
   Deny,
+  // Effectively block the DLL from loading by redirecting its DllMain
+  // to a stub version. This is needed for DLLs that add themselves to
+  // the executable's Import Table, since failing to load would mean the
+  // executable would fail to launch.
   NoOpEntryPoint,
 };
 
@@ -241,12 +254,9 @@ static BOOL WINAPI NoOp_DllMain(HINSTANCE, DWORD, LPVOID) { return TRUE; }
 // in the executable's Import Table.  Because an injected module's
 // DllMain may revert the Import Table to the original state, we parse
 // the Import Table every time a module is loaded without creating a cache.
-static bool IsDependentModule(
+static bool IsInjectedDependentModule(
     const UNICODE_STRING& aModuleLeafName,
     mozilla::freestanding::Kernel32ExportsSolver& aK32Exports) {
-  // We enable automatic DLL blocking only in early Beta or earlier for now
-  // because it caused a compat issue (bug 1682304 and 1704373).
-#if defined(EARLY_BETA_OR_EARLIER)
   mozilla::nt::PEHeaders exeHeaders(aK32Exports.mGetModuleHandleW(nullptr));
   if (!exeHeaders || !exeHeaders.IsImportDirectoryTampered()) {
     // If no tampering is detected, no need to enumerate the Import Table.
@@ -269,9 +279,6 @@ static bool IsDependentModule(
                            &aModuleLeafName, &depModuleLeafName, TRUE) == 0);
       });
   return isDependent;
-#else
-  return false;
-#endif
 }
 
 // Allowing a module to be loaded but detour the entrypoint to NoOp_DllMain
@@ -409,7 +416,7 @@ NTSTATUS NTAPI patched_NtMapViewOfSection(
   UNICODE_STRING leafOnStack;
   nt::GetLeafName(&leafOnStack, sectionFileName);
 
-  bool isDependent = false;
+  bool isInjectedDependent = false;
   const UNICODE_STRING k32Name = MOZ_LITERAL_UNICODE_STRING(L"kernel32.dll");
   Kernel32ExportsSolver* k32Exports = nullptr;
   BlockAction blockAction;
@@ -421,24 +428,53 @@ NTSTATUS NTAPI patched_NtMapViewOfSection(
   } else {
     k32Exports = gSharedSection.GetKernel32Exports();
     // Small optimization: Since loading a dependent module does not involve
-    // LdrLoadDll, we know isDependent is false if we hold a top frame.
+    // LdrLoadDll, we know isInjectedDependent is false if we hold a top frame.
     if (k32Exports && !ModuleLoadFrame::ExistsTopFrame()) {
-      isDependent = IsDependentModule(leafOnStack, *k32Exports);
+      // Note that if a module is dependent but not injected, this means that
+      // the executable built against it, and it should be signed by Mozilla
+      // or Microsoft, so we don't need to worry about adding it to the list
+      // for CIG. (and users won't be able to block it) So the only special
+      // case here is a dependent module that has been injected.
+      isInjectedDependent = IsInjectedDependentModule(leafOnStack, *k32Exports);
     }
 
-    if (isDependent) {
+    if (isInjectedDependent) {
       // Add an NT dv\path to the shared section so that a sandbox process can
       // use it to bypass CIG.  In a sandbox process, this addition fails
       // because we cannot map the section to a writable region, but it's
       // ignorable because the paths have been added by the browser process.
       Unused << gSharedSection.AddDependentModule(sectionFileName);
 
-      // For a dependent module, try redirection instead of blocking it.
+      bool attemptToBlockViaRedirect;
-      // If we fail, we reluctantly allow the module for free.
+#if defined(NIGHTLY_BUILD)
-      mozilla::nt::PEHeaders headers(*aBaseAddress);
+      // We enable automatic DLL blocking only in Nightly for now
-      blockAction = RedirectToNoOpEntryPoint(headers, *k32Exports)
+      // because it caused a compat issue (bug 1682304 and 1704373).
-                        ? BlockAction::NoOpEntryPoint
+      attemptToBlockViaRedirect = true;
-                        : BlockAction::Allow;
+      // We will set blockAction below in the if (attemptToBlockViaRedirect)
+      // block, but I guess the compiler isn't smart enough to figure
+      // that out and complains about an uninitialized variable :-(
+      blockAction = BlockAction::NoOpEntryPoint;
+#else
+      // Check blocklist
+      blockAction =
+          DetermineBlockAction(leafOnStack, *aBaseAddress, k32Exports);
+      // If we were going to block this dependent module, try redirection
+      // instead of blocking it, since blocking it would cause the .exe not to
+      // launch.
+      // Note tht Deny and Error both end up blocking the module in a
+      // straightforward way, so those are the cases in which we need
+      // to redirect instead.
+      attemptToBlockViaRedirect =
+          blockAction == BlockAction::Deny || blockAction == BlockAction::Error;
+#endif
+      if (attemptToBlockViaRedirect) {
+        // For a dependent module, try redirection instead of blocking it.
+        // If we fail, we reluctantly allow the module for free.
+        mozilla::nt::PEHeaders headers(*aBaseAddress);
+        blockAction = RedirectToNoOpEntryPoint(headers, *k32Exports)
+                          ? BlockAction::NoOpEntryPoint
+                          : BlockAction::Allow;
+      }
     } else {
       // Check blocklist
       blockAction =
@@ -476,7 +512,7 @@ NTSTATUS NTAPI patched_NtMapViewOfSection(
   if (nt::RtlGetProcessHeap()) {
     ModuleLoadFrame::NotifySectionMap(
         nt::AllocatedUnicodeString(sectionFileName), *aBaseAddress, stubStatus,
-        loadStatus, isDependent);
+        loadStatus, isInjectedDependent);
   }
 
   if (loadStatus == ModuleLoadInfo::Status::Loaded ||

security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp

@@ -430,7 +430,7 @@ static void AddCachedDirRule(sandbox::TargetPolicy* aPolicy,
 static const Maybe<Vector<const wchar_t*>>& GetPrespawnCigExceptionModules() {
   // We enable pre-spawn CIG only in Nightly for now
   // because it caused a compat issue (bug 1682304 and 1704373).
-#if defined(NIGHTLY)
+#if defined(NIGHTLY_BUILD)
   // The shared section contains a list of dependent modules as a
   // null-delimited string.  We convert it to a string vector and
   // cache it to avoid converting the same data every time.