commit a8c9dab64432c0494c803ccabd7b9cacef75adae
parent 03ade94314b7318be249b7bf3e0c6d7e81c7c581
Author: Cristina Horotan <chorotan@mozilla.com>
Date: Mon, 15 Dec 2025 13:00:32 +0200
Revert "Bug 1998650 - Only favor system DLLs if we can guarantee compatibility. r=bobowen,win-reviewers,gstoll" for causing build bustage at LauncherProcessWin.cpp
This reverts commit 1bfdebd5b9ce46f26d83b25bb3ae177573a12235.
Diffstat:
5 files changed, 42 insertions(+), 164 deletions(-)
diff --git a/browser/app/winlauncher/LauncherProcessWin.cpp b/browser/app/winlauncher/LauncherProcessWin.cpp
@@ -10,6 +10,7 @@
#include "mozilla/CmdLineAndEnvUtils.h"
#include "mozilla/DebugOnly.h"
+#include "mozilla/DynamicallyLinkedFunctionPtr.h"
#include "mozilla/glue/Debug.h"
#include "mozilla/GeckoArgs.h"
#include "mozilla/Maybe.h"
@@ -17,13 +18,12 @@
#include "mozilla/SafeMode.h"
#include "mozilla/UniquePtr.h"
#include "mozilla/WindowsConsole.h"
-#include "mozilla/WindowsProcessMitigations.h"
+#include "mozilla/WindowsVersion.h"
#include "mozilla/WinHeaderOnlyUtils.h"
#include "nsWindowsHelpers.h"
#include <windows.h>
#include <processthreadsapi.h>
-#include <shlwapi.h>
#include "DllBlocklistInit.h"
#include "ErrorHandler.h"
@@ -110,82 +110,16 @@ static nsReturnRef<HANDLE> CreateJobAndAssignProcess(HANDLE aProcess) {
return job.out();
}
-enum class VCRuntimeDLLDir : bool {
- Application,
- System,
-};
-static bool GetMSVCP140VersionInfo(VCRuntimeDLLDir aDir,
- uint64_t& aOutVersion) {
- wchar_t dllPath[MAX_PATH];
- if (aDir == VCRuntimeDLLDir::Application) {
- DWORD size = ::GetModuleFileNameW(nullptr, dllPath, MAX_PATH);
- if (!size ||
- (size == MAX_PATH && ::GetLastError() == ERROR_INSUFFICIENT_BUFFER) ||
- !::PathRemoveFileSpecW(dllPath)) {
- return false;
- }
- } else {
- MOZ_ASSERT(aDir == VCRuntimeDLLDir::System);
- UINT size = ::GetSystemDirectoryW(dllPath, MAX_PATH);
- if (!size || size >= MAX_PATH) {
- return false;
- }
- }
-
- if (!::PathAppendW(dllPath, L"msvcp140.dll")) {
- return false;
- }
- HMODULE crt =
- ::LoadLibraryExW(dllPath, nullptr, LOAD_LIBRARY_AS_IMAGE_RESOURCE);
- if (!crt) {
- return false;
- }
-
- mozilla::nt::PEHeaders headers{crt};
- bool result = headers.GetVersionInfo(aOutVersion);
- ::FreeLibrary(crt);
- return result;
-}
-
-/**
- * Choose whether we want to favor loading DLLs from the system directory over
- * the application directory. This choice automatically propagates to all child
- * processes. In particular, it determines whether child processes will load
- * Visual C++ runtime DLLs from the system or the application directory at
- * startup.
- *
- * Whenever possible, we want all processes to favor loading DLLs from the
- * system directory. But if old Visual C++ runtime DLLs are installed
- * system-wide, then we must favor loading from the application directory
- * instead to ensure compatibility, at least during startup. So in this case we
- * only apply the delayed variant of the mitigation and only in sandboxed
- * processes, which is the best compromise (see SandboxBroker::LaunchApp).
- *
- * This function is called from the launcher process *and* the browser process.
- * This is because if the launcher process is disabled, we still want the
- * browser process to go through this code so that it enforces the correct
- * choice for itself and for child processes.
- */
-static void EnablePreferLoadFromSystem32IfCompatible() {
- // We may already have the mitigation if we are the browser process and we
- // inherited it from the launcher process.
- if (!mozilla::IsPreferLoadFromSystem32Available() ||
- mozilla::IsPreferLoadFromSystem32Enabled()) {
- return;
- }
+#if !defined( \
+ PROCESS_CREATION_MITIGATION_POLICY_IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON)
+# define PROCESS_CREATION_MITIGATION_POLICY_IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON \
+ (0x00000001ULL << 60)
+#endif // !defined(PROCESS_CREATION_MITIGATION_POLICY_IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON)
- // Only bail out if (1) there is a conflict because the two DLLs exist *and*
- // (2) the version of the system DLL is problematic.
- uint64_t systemDirVersion = 0, appDirVersion = 0;
- if (GetMSVCP140VersionInfo(VCRuntimeDLLDir::System, systemDirVersion) &&
- GetMSVCP140VersionInfo(VCRuntimeDLLDir::Application, appDirVersion) &&
- systemDirVersion < appDirVersion) {
- return;
- }
-
- mozilla::DebugOnly<bool> setOk = mozilla::EnablePreferLoadFromSystem32();
- MOZ_ASSERT(setOk);
-}
+#if !defined(PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_OFF)
+# define PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_OFF \
+ (0x00000002ULL << 40)
+#endif // !defined(PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_OFF)
/**
* Any mitigation policies that should be set on the browser process should go
@@ -193,11 +127,10 @@ static void EnablePreferLoadFromSystem32IfCompatible() {
*/
static void SetMitigationPolicies(mozilla::ProcThreadAttributes& aAttrs,
const bool aIsSafeMode) {
- // Note: Do *not* handle IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON here. For this
- // mitigation we rely on EnablePreferLoadFromSystem32IfCompatible().
- // The launcher process or the browser process will choose whether we
- // want to apply the mitigation or not, and child processes will
- // automatically inherit that choice.
+ if (mozilla::IsWin10AnniversaryUpdateOrLater()) {
+ aAttrs.AddMitigationPolicy(
+ PROCESS_CREATION_MITIGATION_POLICY_IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON);
+ }
#if defined(_M_ARM64)
// Disable CFG on older versions of ARM64 Windows to avoid a crash in COM.
@@ -344,9 +277,6 @@ Maybe<int> LauncherMain(int& argc, wchar_t* argv[]) {
return Nothing();
}
- // Called from the launcher process *and* the browser process.
- EnablePreferLoadFromSystem32IfCompatible();
-
#if defined(MOZ_LAUNCHER_PROCESS)
LauncherRegistryInfo regInfo;
Maybe<bool> runAsLauncher = RunAsLauncherProcess(regInfo, argc, argv);
@@ -370,6 +300,22 @@ Maybe<int> LauncherMain(int& argc, wchar_t* argv[]) {
return Nothing();
}
+ // Make sure that the launcher process itself has image load policies set
+ if (IsWin10AnniversaryUpdateOrLater()) {
+ static const StaticDynamicallyLinkedFunctionPtr<
+ decltype(&SetProcessMitigationPolicy)>
+ pSetProcessMitigationPolicy(L"kernel32.dll",
+ "SetProcessMitigationPolicy");
+ if (pSetProcessMitigationPolicy) {
+ PROCESS_MITIGATION_IMAGE_LOAD_POLICY imgLoadPol = {};
+ imgLoadPol.PreferSystem32Images = 1;
+
+ DebugOnly<BOOL> setOk = pSetProcessMitigationPolicy(
+ ProcessImageLoadPolicy, &imgLoadPol, sizeof(imgLoadPol));
+ MOZ_ASSERT(setOk);
+ }
+ }
+
#if defined(MOZ_SANDBOX)
// Ensure the relevant mitigations are enforced.
mozilla::sandboxing::ApplyParentProcessMitigations();
diff --git a/browser/app/winlauncher/moz.build b/browser/app/winlauncher/moz.build
@@ -24,7 +24,6 @@ OS_LIBS += [
"oleaut32",
"ole32",
"rpcrt4",
- "shlwapi",
"version",
]
diff --git a/mozglue/misc/WindowsProcessMitigations.cpp b/mozglue/misc/WindowsProcessMitigations.cpp
@@ -9,7 +9,6 @@
#include <processthreadsapi.h>
#include "mozilla/DynamicallyLinkedFunctionPtr.h"
-#include "mozilla/WindowsVersion.h"
static_assert(sizeof(PROCESS_MITIGATION_DYNAMIC_CODE_POLICY) == 4);
@@ -98,54 +97,4 @@ MFBT_API bool IsUserShadowStackEnabled() {
return polInfo.EnableUserShadowStack;
}
-MFBT_API bool IsPreferLoadFromSystem32Available() {
- return mozilla::IsWin10AnniversaryUpdateOrLater();
-}
-
-MFBT_API bool IsPreferLoadFromSystem32Enabled() {
- auto pGetProcessMitigationPolicy = FetchGetProcessMitigationPolicyFunc();
- if (!pGetProcessMitigationPolicy) {
- return false;
- }
-
- PROCESS_MITIGATION_IMAGE_LOAD_POLICY imgLoadPol{};
- if (!pGetProcessMitigationPolicy(::GetCurrentProcess(),
- ProcessImageLoadPolicy, &imgLoadPol,
- sizeof(imgLoadPol))) {
- return false;
- }
-
- return imgLoadPol.PreferSystem32Images;
-}
-
-MFBT_API bool EnablePreferLoadFromSystem32() {
- auto pGetProcessMitigationPolicy = FetchGetProcessMitigationPolicyFunc();
- if (!pGetProcessMitigationPolicy) {
- return false;
- }
-
- static const mozilla::StaticDynamicallyLinkedFunctionPtr<
- decltype(&::SetProcessMitigationPolicy)>
- pSetProcessMitigationPolicy(L"kernel32.dll",
- "SetProcessMitigationPolicy");
- if (!pSetProcessMitigationPolicy) {
- return false;
- }
-
- PROCESS_MITIGATION_IMAGE_LOAD_POLICY imgLoadPol{};
- if (!pGetProcessMitigationPolicy(::GetCurrentProcess(),
- ProcessImageLoadPolicy, &imgLoadPol,
- sizeof(imgLoadPol))) {
- return false;
- }
-
- if (imgLoadPol.PreferSystem32Images) {
- return true;
- }
-
- imgLoadPol.PreferSystem32Images = 1;
- return pSetProcessMitigationPolicy(ProcessImageLoadPolicy, &imgLoadPol,
- sizeof(imgLoadPol));
-}
-
} // namespace mozilla
diff --git a/mozglue/misc/WindowsProcessMitigations.h b/mozglue/misc/WindowsProcessMitigations.h
@@ -16,9 +16,6 @@ MFBT_API void SetWin32kLockedDownInPolicy();
MFBT_API bool IsDynamicCodeDisabled();
MFBT_API bool IsEafPlusEnabled();
MFBT_API bool IsUserShadowStackEnabled();
-MFBT_API bool IsPreferLoadFromSystem32Available();
-MFBT_API bool IsPreferLoadFromSystem32Enabled();
-MFBT_API bool EnablePreferLoadFromSystem32();
} // namespace mozilla
diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -471,36 +471,19 @@ Result<Ok, mozilla::ipc::LaunchError> SandboxBroker::LaunchApp(
"Setting the reduced set of flags should always succeed");
}
- sandbox::MitigationFlags delayedMitigations =
- config->GetDelayedProcessMitigations();
-
- // Only prefer loading from the system directory as a delayed mitigation, and
- // always enable this delayed mitigation. This means that:
- // - if the launcher or browser process chose to apply the mitigation, child
- // processes will have it enabled at startup automatically anyway;
- // - even if the launcher or browser process chose not to apply the
- // mitigation, at least sandboxed child processes will run with the
- // mitigation once the sandbox starts (by this time, they will already
- // have loaded the Visual C++ runtime DLLs, so these are no longer a
- // concern; also, although some sandboxed child processes can start new
- // processes, they never start new *Firefox* processes).
- // Refer to EnablePreferLoadFromSystem32IfCompatible for more details.
- MOZ_ASSERT(!(config->GetProcessMitigations() &
- sandbox::MITIGATION_IMAGE_LOAD_PREFER_SYS32));
- delayedMitigations |= sandbox::MITIGATION_IMAGE_LOAD_PREFER_SYS32;
-
// Bug 1936749: MpDetours.dll injection is incompatible with ACG.
constexpr sandbox::MitigationFlags kDynamicCodeFlags =
sandbox::MITIGATION_DYNAMIC_CODE_DISABLE |
sandbox::MITIGATION_DYNAMIC_CODE_DISABLE_WITH_OPT_OUT;
+ sandbox::MitigationFlags delayedMitigations =
+ config->GetDelayedProcessMitigations();
if ((delayedMitigations & kDynamicCodeFlags) &&
::GetModuleHandleW(L"MpDetours.dll")) {
delayedMitigations &= ~kDynamicCodeFlags;
+ SANDBOX_SUCCEED_OR_CRASH(
+ config->SetDelayedProcessMitigations(delayedMitigations));
}
- SANDBOX_SUCCEED_OR_CRASH(
- config->SetDelayedProcessMitigations(delayedMitigations));
-
EnsureAppLockerAccess(config);
// If logging enabled, set up the policy.
@@ -1125,7 +1108,8 @@ void SandboxBroker::SetSecurityLevelForContentProcess(int32_t aSandboxLevel,
sandbox::MITIGATION_DEP | sandbox::MITIGATION_EXTENSION_POINT_DISABLE |
sandbox::MITIGATION_KTM_COMPONENT | sandbox::MITIGATION_FSCTL_DISABLED |
sandbox::MITIGATION_IMAGE_LOAD_NO_REMOTE |
- sandbox::MITIGATION_IMAGE_LOAD_NO_LOW_LABEL;
+ sandbox::MITIGATION_IMAGE_LOAD_NO_LOW_LABEL |
+ sandbox::MITIGATION_IMAGE_LOAD_PREFER_SYS32;
#if defined(_M_ARM64)
// Disable CFG on older versions of ARM64 Windows to avoid a crash in COM.
@@ -1446,7 +1430,8 @@ bool SandboxBroker::SetSecurityLevelForRDDProcess() {
sandbox::MITIGATION_NONSYSTEM_FONT_DISABLE |
sandbox::MITIGATION_KTM_COMPONENT | sandbox::MITIGATION_FSCTL_DISABLED |
sandbox::MITIGATION_IMAGE_LOAD_NO_REMOTE |
- sandbox::MITIGATION_IMAGE_LOAD_NO_LOW_LABEL;
+ sandbox::MITIGATION_IMAGE_LOAD_NO_LOW_LABEL |
+ sandbox::MITIGATION_IMAGE_LOAD_PREFER_SYS32;
if (StaticPrefs::security_sandbox_rdd_shadow_stack_enabled()) {
mitigations |= sandbox::MITIGATION_CET_COMPAT_MODE;
@@ -1529,7 +1514,8 @@ bool SandboxBroker::SetSecurityLevelForSocketProcess() {
sandbox::MITIGATION_NONSYSTEM_FONT_DISABLE |
sandbox::MITIGATION_KTM_COMPONENT | sandbox::MITIGATION_FSCTL_DISABLED |
sandbox::MITIGATION_IMAGE_LOAD_NO_REMOTE |
- sandbox::MITIGATION_IMAGE_LOAD_NO_LOW_LABEL;
+ sandbox::MITIGATION_IMAGE_LOAD_NO_LOW_LABEL |
+ sandbox::MITIGATION_IMAGE_LOAD_PREFER_SYS32;
if (StaticPrefs::security_sandbox_socket_shadow_stack_enabled()) {
mitigations |= sandbox::MITIGATION_CET_COMPAT_MODE;
@@ -1601,6 +1587,7 @@ struct UtilitySandboxProps {
sandbox::MITIGATION_KTM_COMPONENT | sandbox::MITIGATION_FSCTL_DISABLED |
sandbox::MITIGATION_IMAGE_LOAD_NO_REMOTE |
sandbox::MITIGATION_IMAGE_LOAD_NO_LOW_LABEL |
+ sandbox::MITIGATION_IMAGE_LOAD_PREFER_SYS32 |
sandbox::MITIGATION_CET_COMPAT_MODE;
sandbox::MitigationFlags mExcludedInitialMitigations = 0;
