From 3adaf2da2306d13857e560d3ae863621946f230a Mon Sep 17 00:00:00 2001
From: S Ganesh <ganesh@chromium.org>
Date: Mon, 27 Apr 2026 20:44:45 -0700
Subject: [PATCH] [elevation_service] Add version check for chrome recovery

This change adds rollback protection for chrome recovery by reading the
CRX manifest after unpacking and rejecting packages with a version
older than the currently running Chrome instance.

Fixed: 500483038
Change-Id: Ic8d98662bf5c991c4a9ae7918324f216d10fd894
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7793834
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Commit-Queue: S Ganesh <ganesh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1621527}
---

diff --git a/chrome/browser/component_updater/recovery_improved_component_installer_win.cc b/chrome/browser/component_updater/recovery_improved_component_installer_win.cc
index 4711618..f4844f6a 100644
--- a/chrome/browser/component_updater/recovery_improved_component_installer_win.cc
+++ b/chrome/browser/component_updater/recovery_improved_component_installer_win.cc
@@ -25,6 +25,7 @@
 #include "chrome/browser/component_updater/recovery_improved_component_installer.h"
 #include "chrome/elevation_service/elevation_service_idl.h"
 #include "chrome/install_static/install_util.h"
+#include "chrome/installer/util/util_constants.h"
 #include "components/version_info/version_info.h"
 
 namespace component_updater {
@@ -114,7 +115,8 @@
 base::CommandLine RecoveryComponentActionHandlerWin::MakeCommandLine(
     const base::FilePath& unpack_path) const {
   base::CommandLine command_line(unpack_path.Append(kRecoveryFileName));
-  command_line.AppendSwitchASCII("browser-version", GetBrowserVersion());
+  command_line.AppendSwitchASCII(installer::switches::kBrowserVersionSwitch,
+                                 GetBrowserVersion());
   command_line.AppendSwitchASCII("sessionid", session_id());
   const auto app_guid = GetBrowserAppId();
   if (!app_guid.empty()) {
diff --git a/chrome/elevation_service/BUILD.gn b/chrome/elevation_service/BUILD.gn
index 0ce72b2..11ec3c4d 100644
--- a/chrome/elevation_service/BUILD.gn
+++ b/chrome/elevation_service/BUILD.gn
@@ -66,6 +66,7 @@
     "//chrome/installer/util:isolation_support",
     "//chrome/windows_services/service_program:common",
     "//components/crx_file",
+    "//components/update_client",
     "//content/public/common:static_switches",
     "//third_party/zlib/google:zip",
   ]
diff --git a/chrome/elevation_service/DEPS b/chrome/elevation_service/DEPS
index 8a35a16d5..edae50163 100644
--- a/chrome/elevation_service/DEPS
+++ b/chrome/elevation_service/DEPS
@@ -3,5 +3,6 @@
   "+chrome/installer/util",
   "+chrome/windows_services/service_program",
   "+components/crx_file",
+  "+components/update_client",
   "+third_party/zlib",
 ]
diff --git a/chrome/elevation_service/elevated_recovery_impl.cc b/chrome/elevation_service/elevated_recovery_impl.cc
index 74a1b235..60bfb51 100644
--- a/chrome/elevation_service/elevated_recovery_impl.cc
+++ b/chrome/elevation_service/elevated_recovery_impl.cc
@@ -23,10 +23,14 @@
 #include "base/process/process.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/version.h"
+#include "base/win/registry.h"
 #include "base/win/scoped_process_information.h"
+#include "chrome/install_static/install_modes.h"
 #include "chrome/install_static/install_util.h"
+#include "chrome/installer/util/util_constants.h"
 #include "chrome/windows_services/service_program/scoped_client_impersonation.h"
 #include "components/crx_file/crx_verifier.h"
+#include "components/update_client/utils.h"
 #include "third_party/zlib/google/zip.h"
 
 namespace elevation_service {
@@ -317,10 +321,41 @@
   if (FAILED(hr))
     return hr;
 
+  // Read version autonomously from secured HKLM machine registries based on
+  // AppID.
+  base::win::RegKey key(
+      HKEY_LOCAL_MACHINE,
+      install_static::GetClientsKeyPath(browser_appid.c_str()).c_str(),
+      KEY_QUERY_VALUE);
+  std::wstring registry_version;
+  if (key.ReadValue(FILE_PATH_LITERAL("version"), &registry_version) !=
+      ERROR_SUCCESS) {
+    // Fall back on RPC caller version if registry read fails. Registry keys
+    // may be missing or corrupted on severely broken environments that recovery
+    // specifically targets. Note that deliberately modifying the HKLM version
+    // is something that only administrators can do, and medium integrity
+    // attackers cannot natively bypass floor checks by clearing those keys.
+    registry_version = browser_version;
+  }
+  const base::Version registry_version_parsed(
+      base::WideToASCII(registry_version));
+  if (!registry_version_parsed.IsValid()) {
+    return E_FAIL;
+  }
+
+  // Trapping attacks by returning E_ACCESSDENIED on discrepancies.
+  const base::Version browser_version_parsed(
+      base::WideToASCII(browser_version));
+  if (!browser_version_parsed.IsValid() ||
+      browser_version_parsed != registry_version_parsed) {
+    return E_ACCESSDENIED;
+  }
+
   base::CommandLine args(base::CommandLine::NO_PROGRAM);
   if (!browser_appid.empty())
     args.AppendSwitchNative("appguid", browser_appid);
-  args.AppendSwitchNative("browser-version", browser_version);
+  args.AppendSwitchNative(installer::switches::kBrowserVersionSwitch,
+                          browser_version);
   args.AppendSwitchNative("sessionid", session_id);
   args.AppendSwitch("system");
 
@@ -329,7 +364,7 @@
   if (FAILED(hr))
     return hr;
 
-  return RunCRX(crx_path, args,
+  return RunCRX(crx_path, args, browser_version_parsed,
                 crx_file::VerifierFormat::CRX3_WITH_PUBLISHER_PROOF,
                 GetRecoveryCRXHash(), unpack_dir,
                 base::FilePath(kRecoveryExeName), caller_proc_id, proc_handle);
@@ -337,6 +372,7 @@
 
 HRESULT RunCRX(const base::FilePath& crx_path,
                const base::CommandLine& args,
+               const base::Version& min_crx_version,
                const crx_file::VerifierFormat& crx_format,
                const std::vector<uint8_t>& crx_hash,
                const base::FilePath& unpack_under_path,
@@ -361,6 +397,24 @@
   if (FAILED(hr))
     return hr;
 
+  const auto manifest = update_client::ReadManifest(unpacked_crx_dir.GetPath());
+  if (!manifest) {
+    return E_FAIL;
+  }
+  const std::string* manifest_version_str = manifest->FindString("version");
+  if (!manifest_version_str) {
+    return E_FAIL;
+  }
+  const base::Version manifest_version(*manifest_version_str);
+  if (!manifest_version.IsValid()) {
+    return E_FAIL;
+  }
+
+  // Trapping attacks by mapping E_ACCESSDENIED on rollbacks.
+  if (manifest_version < min_crx_version) {
+    return E_ACCESSDENIED;
+  }
+
   const base::FilePath path_and_name =
       unpacked_crx_dir.GetPath().Append(exe_filename);
   base::CommandLine command_line(path_and_name);
diff --git a/chrome/elevation_service/elevated_recovery_impl.h b/chrome/elevation_service/elevated_recovery_impl.h
index 64ef31b..d7012a0 100644
--- a/chrome/elevation_service/elevated_recovery_impl.h
+++ b/chrome/elevation_service/elevated_recovery_impl.h
@@ -16,6 +16,7 @@
 
 class CommandLine;
 class FilePath;
+class Version;
 
 }  // namespace base
 
@@ -45,8 +46,11 @@
 // |crx_path|. The returned |proc_handle| is a process handle that is valid for
 // the |caller_proc_id| process, or the current process if |caller_proc_id| is
 // 0. |unpacked_under_path| is expected to be eventually deleted by the caller.
+// |min_crx_version| represents the minimum version of the CRX manifest allowed,
+// to prevent version rollback attacks.
 HRESULT RunCRX(const base::FilePath& crx_path,
                const base::CommandLine& args,
+               const base::Version& min_crx_version,
                const crx_file::VerifierFormat& crx_format,
                const std::vector<uint8_t>& crx_hash,
                const base::FilePath& unpack_under_path,
diff --git a/chrome/elevation_service/elevated_recovery_unittest.cc b/chrome/elevation_service/elevated_recovery_unittest.cc
index dfd5fe7..f36d0a15 100644
--- a/chrome/elevation_service/elevated_recovery_unittest.cc
+++ b/chrome/elevation_service/elevated_recovery_unittest.cc
@@ -14,6 +14,7 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
+#include "base/version.h"
 #include "base/win/scoped_com_initializer.h"

diff --git a/chrome/elevation_service/elevated_recovery_unittest.cc b/chrome/elevation_service/elevated_recovery_unittest.cc
index dfd5fe7..f36d0a15 100644
--- a/chrome/elevation_service/elevated_recovery_unittest.cc
+++ b/chrome/elevation_service/elevated_recovery_unittest.cc
@@ -14,6 +14,7 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
+#include "base/version.h"
 #include "base/win/scoped_com_initializer.h"
 #include "chrome/elevation_service/elevated_recovery_impl.h"
 #include "chrome/windows_services/service_program/test_support/scoped_mock_context.h"
@@ -121,33 +122,35 @@
 
 TEST_F(ElevatedRecoveryTest, Do_RunCRX_InvalidArgs) {
   base::win::ScopedHandle proc_handle;
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  command_line.AppendSwitchNative("browser-version", L"0.0.0.0");
 
   // Non-matching CRX/CRX-hash.
   EXPECT_EQ(CRYPT_E_NO_MATCH,
             elevation_service::RunCRX(
-                TestFile("valid_no_publisher.crx3"),
-                base::CommandLine(base::CommandLine::NO_PROGRAM),
+                TestFile("valid_no_publisher.crx3"), command_line,
+                base::Version("0.0.0.0"),
                 crx_file::VerifierFormat::CRX3_WITH_PUBLISHER_PROOF,
                 GetValidPublisherCrx3Hash(), GetUnpackDir(),
                 base::FilePath(kManifestJSONFileName), ::GetCurrentProcessId(),
                 &proc_handle));
 
   // Non-existent CRX file.
-  EXPECT_EQ(HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND),
-            elevation_service::RunCRX(
-                TestFile("nonexistent.crx3"),
-                base::CommandLine(base::CommandLine::NO_PROGRAM),
-                crx_file::VerifierFormat::CRX3_WITH_PUBLISHER_PROOF,
-                GetValidPublisherCrx3Hash(), GetUnpackDir(),
-                base::FilePath(kManifestJSONFileName), ::GetCurrentProcessId(),
-                &proc_handle));
+  EXPECT_EQ(
+      HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND),
+      elevation_service::RunCRX(
+          TestFile("nonexistent.crx3"), command_line, base::Version("0.0.0.0"),
+          crx_file::VerifierFormat::CRX3_WITH_PUBLISHER_PROOF,
+          GetValidPublisherCrx3Hash(), GetUnpackDir(),
+          base::FilePath(kManifestJSONFileName), ::GetCurrentProcessId(),
+          &proc_handle));
 
   // manifest.json is not a Windows executable, ::CreateProcess therefore
   // returns ERROR_BAD_EXE_FORMAT.
   EXPECT_EQ(HRESULT_FROM_WIN32(ERROR_BAD_EXE_FORMAT),
             elevation_service::RunCRX(
-                TestFile("valid_publisher.crx3"),
-                base::CommandLine(base::CommandLine::NO_PROGRAM),
+                TestFile("valid_publisher.crx3"), command_line,
+                base::Version("0.0.0.0"),
                 crx_file::VerifierFormat::CRX3_WITH_PUBLISHER_PROOF,
                 GetValidPublisherCrx3Hash(), GetUnpackDir(),
                 base::FilePath(kManifestJSONFileName), ::GetCurrentProcessId(),
@@ -156,16 +159,17 @@
 
 TEST_F(ElevatedRecoveryTest, Do_RunCRX_ValidArgs) {
   base::win::ScopedHandle proc_handle;
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  command_line.AppendSwitchNative("browser-version", L"0.0.0.0");
 
   // ChromeRecovery.crx3 contains ChromeRecovery.exe which returns a hardcoded
   // value of 1877345072.
-  EXPECT_EQ(S_OK,
-            elevation_service::RunCRX(
-                TestFile("ChromeRecovery.crx3"),
-                base::CommandLine(base::CommandLine::NO_PROGRAM),
-                crx_file::VerifierFormat::CRX3, GetRunactionTestWinCrx3Hash(),
-                GetUnpackDir(), base::FilePath(kRecoveryExeName),
-                ::GetCurrentProcessId(), &proc_handle));
+  EXPECT_EQ(S_OK, elevation_service::RunCRX(
+                      TestFile("ChromeRecovery.crx3"), command_line,
+                      base::Version("0.0.0.0"), crx_file::VerifierFormat::CRX3,
+                      GetRunactionTestWinCrx3Hash(), GetUnpackDir(),
+                      base::FilePath(kRecoveryExeName), ::GetCurrentProcessId(),
+                      &proc_handle));
 
   EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(proc_handle.Get(), 500));
   DWORD exit_code = 0;
@@ -173,6 +177,28 @@
   EXPECT_EQ(1877345072UL, exit_code);
 }
 
+TEST_F(ElevatedRecoveryTest, Do_RunCRX_VersionCheck) {
+  base::win::ScopedHandle proc_handle;
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+
+  // Trapping attacks by mapping E_ACCESSDENIED on rollbacks.
+  EXPECT_EQ(E_ACCESSDENIED,
+            elevation_service::RunCRX(
+                TestFile("ChromeRecovery.crx3"), command_line,
+                base::Version("1000.0.0.0"), crx_file::VerifierFormat::CRX3,
+                GetRunactionTestWinCrx3Hash(), GetUnpackDir(),
+                base::FilePath(kRecoveryExeName), ::GetCurrentProcessId(),
+                &proc_handle));
+
+  // Verification handling successfully passing low floors.
+  EXPECT_EQ(S_OK, elevation_service::RunCRX(
+                      TestFile("ChromeRecovery.crx3"), command_line,
+                      base::Version("0.0.0.0"), crx_file::VerifierFormat::CRX3,
+                      GetRunactionTestWinCrx3Hash(), GetUnpackDir(),
+                      base::FilePath(kRecoveryExeName), ::GetCurrentProcessId(),
+                      &proc_handle));
+}
+
 TEST(ElevatedRecoveryCleanupTest, Do_CleanupChromeRecoveryDirectory) {
   base::FilePath recovery_dir;
   ASSERT_TRUE(base::PathService::Get(base::DIR_EXE, &recovery_dir));