Chrome · Updater
CVE-2026-17861
Logic Error in Updater
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifchrome/updater/test/integration_tests.cc |
modified | |
TEST_Fchrome/updater/test/integration_tests.cc |
modified |
Files Changed
chrome/updater/app/server/win/com_classes_util.ccchrome/updater/test/integration_tests.ccchrome/updater/test/integration_tests_impl.ccchrome/updater/update_service_impl_impl.cc
Patch
From de524c47071913e267de97ff566dde7852ec7cfc Mon Sep 17 00:00:00 2001 From: Noah Rose Ledesma <[email protected]> Date: Wed, 17 Jun 2026 14:12:49 -0700 Subject: [PATCH] Add additional validation checks for App IDs Verify App IDs at the UpdateService / COM entry point. This rejects invalid App IDs containing path separators ('/', '\') or parent directory references ('.', '..') gracefully. Additionally, add fallback sanitation in Windows registry key helper functions to prevent subkey injection if validation is bypassed. Bug: 520417861 Change-Id: Ia63709c60aa178da0d495f4ecff076156a6a6964 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7903226 Auto-Submit: Noah Rose Ledesma <[email protected]> Commit-Queue: Noah Rose Ledesma <[email protected]> Reviewed-by: Sorin Jianu <[email protected]> Commit-Queue: Sorin Jianu <[email protected]> Reviewed-by: S Ganesh <[email protected]> Cr-Commit-Position: refs/heads/main@{#1648581} --- diff --git a/chrome/updater/app/server/win/com_classes_util.cc b/chrome/updater/app/server/win/com_classes_util.cc index 3d184b83..1b796c4f 100644 --- a/chrome/updater/app/server/win/com_classes_util.cc +++ b/chrome/updater/app/server/win/com_classes_util.cc @@ -16,6 +16,7 @@ #include "base/win/windows_types.h" #include "chrome/updater/get_updater_scope.h" #include "chrome/updater/registration_data.h" +#include "chrome/updater/util/util.h" #include "chrome/updater/util/win_util.h" namespace updater { @@ -65,7 +66,10 @@ } std::optional<std::string> ValidateAppId(const wchar_t* app_id) { - return ValidateStringEmptyNotOk(app_id, kMaxStringLen); + std::optional<std::string> app_id_s = + ValidateStringEmptyNotOk(app_id, kMaxStringLen); + return app_id_s && IsValidAppId(*app_id_s) ? std::move(app_id_s) + : std::nullopt; } std::optional<std::string> ValidateCommandId(const wchar_t* command_id) { diff --git a/chrome/updater/test/integration_tests.cc b/chrome/updater/test/integration_tests.cc index 9afbc5a..1a4eec1 100644 --- a/chrome/updater/test/integration_tests.cc +++ b/chrome/updater/test/integration_tests.cc @@ -134,46 +134,45 @@ if (app_version) { app_expectation.Set("version", app_version->GetString()); } - test_server.ExpectOnce({request::GetUpdaterUserAgentMatcher(updater_version), - request::GetJSONContentMatcher( - base::DictValue().SetByDottedPath( - "request.apps", - base::ListValue().Append( - std::move(app_expectation))))}, - base::BindRepeating( - [](const std::string& app_id, bool v4) { - return v4 ? base::StringPrintf( - ")]}'\n" - R"({"response":{)" - R"( "protocol":"4.0",)" - R"( "apps":[)" - R"( {)" - R"( "appid":"%s",)" - R"( "status":"ok",)" - R"( "updatecheck":{)" - R"( "status":"noupdate")" - R"( })" - R"( })" - R"( ])" - R"(}})", - app_id) - : base::StringPrintf( - ")]}'\n" - R"({"response":{)" - R"( "protocol":"3.1",)" - R"( "app":[)" - R"( {)" - R"( "appid":"%s",)" - R"( "status":"ok",)" - R"( "updatecheck":{)" - R"( "status":"noupdate")" - R"( })" - R"( })" - R"( ])" - R"(}})", - app_id); - }, - app_id)); + test_server.ExpectOnce( + {request::GetUpdaterUserAgentMatcher(updater_version), + request::GetJSONContentMatcher(base::DictValue().SetByDottedPath( + "request.apps", + base::ListValue().Append(std::move(app_expectation))))}, + base::BindRepeating( + [](const std::string& app_id, bool v4) { + return v4 ? base::StringPrintf( + ")]}'\n" + R"({"response":{)" + R"( "protocol":"4.0",)" + R"( "apps":[)" + R"( {)" + R"( "appid":"%s",)" + R"( "status":"ok",)" + R"( "updatecheck":{)" + R"( "status":"noupdate")" + R"( })" + R"( })" + R"( ])" + R"(}})", + app_id) + : base::StringPrintf( + ")]}'\n" + R"({"response":{)" + R"( "protocol":"3.1",)" + R"( "app":[)" + R"( {)" + R"( "appid":"%s",)" + R"( "status":"ok",)" + R"( "updatecheck":{)" + R"( "status":"noupdate")" + R"( })" + R"( })" + R"( ])" + R"(}})", + app_id); + }, + app_id)); } void ExpectPingRequest( @@ -2567,6 +2566,18 @@ ASSERT_NO_FATAL_FAILURE(Uninstall()); } +TEST_F(IntegrationTest, InstallAppInvalidAppId) { + ScopedServer test_server(test_commands_); + ExpectInstallEvent(test_server, kUpdaterAppId); + ASSERT_NO_FATAL_FAILURE(Install()); + + ASSERT_NO_FATAL_FAILURE(InstallAppViaService( + "invalid/appid", base::DictValue().Set("expect_failure", true))); + + ASSERT_NO_FATAL_FAILURE(ExpectUninstallPing(test_server)); + ASSERT_NO_FATAL_FAILURE(Uninstall()); +} + TEST_F(IntegrationTest, CreateCorrectAndIncorrectScopeProxies) { ASSERT_NO_FATAL_FAILURE(Install()); diff --git a/chrome/updater/test/integration_tests_impl.cc b/chrome/updater/test/integration_tests_impl.cc index 7bbdc6b..d78b235 100644 --- a/chrome/updater/test/integration_tests_impl.cc +++ b/chrome/updater/test/integration_tests_impl.cc @@ -1084,6 +1084,8 @@ expected_final_values.FindInt("expected_result"); expected_result) { EXPECT_EQ(static_cast<int>(final_result), *expected_result); + } else if (expected_final_values.FindBool("expect_failure").value_or(false)) { + EXPECT_NE(final_result, UpdateService::Result::kSuccess); } } diff --git a/chrome/updater/update_service_impl_impl.cc b/chrome/updater/update_service_impl_impl.cc index 03a73970..fea4a40 100644 --- a/chrome/updater/update_service_impl_impl.cc +++ b/chrome/updater/update_service_impl_impl.cc @@ -768,8 +768,8 @@ return; } - if (request.app_id.empty()) { - VLOG(1) << "Refusing to register an empty app ID."; + if (!IsValidAppId(request.app_id)) { + VLOG(1) << "Refusing to register an invalid app ID: " << request.app_id; base::SequencedTaskRunner::GetCurrentDefault()->PostTask( FROM_HERE, base::BindOnce(std::move(callback), kRegistrationError)); return; @@ -1002,6 +1002,14 @@ VLOG(1) << __func__ << ": " << app_id; DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + if (!IsValidAppId(app_id)) { + VLOG(1) << "Refusing to check update for an invalid app ID: " << app_id; + base::SequencedTaskRunner::GetCurrentDefault()->PostTask( + FROM_HERE, + base::BindOnce(std::move(callback), Result::kInvalidArgument)); + return; + } + base::MakeRefCounted<HandleInconsistentAppsTask>(config_, GetUpdaterScope()) ->Run(base::BindOnce( &UpdateServiceImplImpl::FetchPolicies, this,
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/updater/test/integration_tests.cc b/chrome/updater/test/integration_tests.cc
index 9afbc5a..1a4eec1 100644
--- a/chrome/updater/test/integration_tests.cc
+++ b/chrome/updater/test/integration_tests.cc
@@ -134,46 +134,45 @@
if (app_version) {
app_expectation.Set("version", app_version->GetString());
}
- test_server.ExpectOnce({request::GetUpdaterUserAgentMatcher(updater_version),
- request::GetJSONContentMatcher(
- base::DictValue().SetByDottedPath(
- "request.apps",
- base::ListValue().Append(
- std::move(app_expectation))))},
- base::BindRepeating(
- [](const std::string& app_id, bool v4) {
- return v4 ? base::StringPrintf(
- ")]}'\n"
- R"({"response":{)"
- R"( "protocol":"4.0",)"
- R"( "apps":[)"
- R"( {)"
- R"( "appid":"%s",)"
- R"( "status":"ok",)"
- R"( "updatecheck":{)"
- R"( "status":"noupdate")"
- R"( })"
- R"( })"
- R"( ])"
- R"(}})",
- app_id)
- : base::StringPrintf(
- ")]}'\n"
- R"({"response":{)"
- R"( "protocol":"3.1",)"
- R"( "app":[)"
- R"( {)"
- R"( "appid":"%s",)"
- R"( "status":"ok",)"
- R"( "updatecheck":{)"
- R"( "status":"noupdate")"
- R"( })"
- R"( })"
- R"( ])"
- R"(}})",
- app_id);
- },
- app_id));
+ test_server.ExpectOnce(
+ {request::GetUpdaterUserAgentMatcher(updater_version),
+ request::GetJSONContentMatcher(base::DictValue().SetByDottedPath(
+ "request.apps",
+ base::ListValue().Append(std::move(app_expectation))))},
+ base::BindRepeating(
+ [](const std::string& app_id, bool v4) {
+ return v4 ? base::StringPrintf(
+ ")]}'\n"
+ R"({"response":{)"
+ R"( "protocol":"4.0",)"
+ R"( "apps":[)"
+ R"( {)"
+ R"( "appid":"%s",)"
+ R"( "status":"ok",)"
+ R"( "updatecheck":{)"
+ R"( "status":"noupdate")"
+ R"( })"
+ R"( })"
+ R"( ])"
+ R"(}})",
+ app_id)
+ : base::StringPrintf(
+ ")]}'\n"
+ R"({"response":{)"
+ R"( "protocol":"3.1",)"
+ R"( "app":[)"
+ R"( {)"
+ R"( "appid":"%s",)"
+ R"( "status":"ok",)"
+ R"( "updatecheck":{)"
+ R"( "status":"noupdate")"
+ R"( })"
+ R"( })"
+ R"( ])"
+ R"(}})",
+ app_id);
+ },
+ app_id));
}
void ExpectPingRequest(
@@ -2567,6 +2566,18 @@
ASSERT_NO_FATAL_FAILURE(Uninstall());
}
+TEST_F(IntegrationTest, InstallAppInvalidAppId) {
+ ScopedServer test_server(test_commands_);
+ ExpectInstallEvent(test_server, kUpdaterAppId);
+ ASSERT_NO_FATAL_FAILURE(Install());
+
+ ASSERT_NO_FATAL_FAILURE(InstallAppViaService(
+ "invalid/appid", base::DictValue().Set("expect_failure", true)));
+
+ ASSERT_NO_FATAL_FAILURE(ExpectUninstallPing(test_server));
+ ASSERT_NO_FATAL_FAILURE(Uninstall());
+}
+
TEST_F(IntegrationTest, CreateCorrectAndIncorrectScopeProxies) {
ASSERT_NO_FATAL_FAILURE(Install());
diff --git a/chrome/updater/test/integration_tests_impl.cc b/chrome/updater/test/integration_tests_impl.cc
index 7bbdc6b..d78b235 100644
--- a/chrome/updater/test/integration_tests_impl.cc
+++ b/chrome/updater/test/integration_tests_impl.cc
@@ -1084,6 +1084,8 @@
expected_final_values.FindInt("expected_result");
expected_result) {
EXPECT_EQ(static_cast<int>(final_result), *expected_result);
+ } else if (expected_final_values.FindBool("expect_failure").value_or(false)) {
+ EXPECT_NE(final_result, UpdateService::Result::kSuccess);
}
}
diff --git a/chrome/updater/util/util_unittest.cc b/chrome/updater/util/util_unittest.cc
index 365254b..2d4a791 100644
--- a/chrome/updater/util/util_unittest.cc
+++ b/chrome/updater/util/util_unittest.cc
@@ -319,4 +319,40 @@
EXPECT_TRUE(base::DeletePathRecursively(dir));
}
+TEST(Util, IsValidAppId) {
+ for (const auto& valid_app_id :
+ {"COM.GOOGLE.CHROME", "{8A69F345-C564-463C-AFF1-A69D9E530F96}"}) {
+ EXPECT_TRUE(IsValidAppId(valid_app_id));
+ EXPECT_TRUE(IsValidAppId(base::UTF8ToWide(valid_app_id)));
+ }
+
+ for (const std::string& invalid_app_id : std::vector<std::string>{
+ "",
+ std::string(257, 'a'),
+ "a/b",
+ "a\\b",
+ "..",
+ ".",
+ "../a",
+ "a/../b",
+ "a\\..\\b",
+ "/",
+ "\\",
+ "/a",
+ "\\a",
+ "a\tb",
+ "a\nb",
+ "a\rb",
+ "a\x01"
+ "b",
+ "a\x7f"
+ "b",
+ "a\xc3\xa9"
+ "b",
+ }) {
+ EXPECT_FALSE(IsValidAppId(invalid_app_id));
+ EXPECT_FALSE(IsValidAppId(base::UTF8ToWide(invalid_app_id)));
+ }
+}
+
} // namespace updater
diff --git a/chrome/updater/util/win_util_unittest.cc b/chrome/updater/util/win_util_unittest.cc
index 6f76915d..af33627 100644
--- a/chrome/updater/util/win_util_unittest.cc
+++ b/chrome/updater/util/win_util_unittest.cc
@@ -38,6 +38,7 @@
#include "base/task/thread_pool.h"
#include "base/test/bind.h"
#include "base/test/gmock_expected_support.h"
+#include "base/test/gtest_util.h"
#include "base/test/task_environment.h"
#include "base/test/test_timeouts.h"
#include "base/threading/platform_thread.h"
@@ -60,10 +61,13 @@
#include "chrome/updater/win/test/test_executables.h"
#include "chrome/updater/win/test/test_strings.h"
#include "chrome/updater/win/win_constants.h"
+#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace updater::test {
+using ::testing::EndsWith;
+
namespace {
constexpr char kTestAppID[] = "{D07D2B56-F583-4631-9E8E-9942F63765BE}";
@@ -862,4 +866,10 @@
base::win::ScopedLocalAlloc sd_holder(raw_sd);
}
+TEST(WinUtil, RegistryKeyHelpersSanitizeInvalidAppIds) {
+ EXPECT_THAT(GetAppClientsKey(L"a\\b"), EndsWith(L"a_b"));
+ EXPECT_THAT(GetAppClientStateKey(L"a\\b"), EndsWith(L"a_b"));
+ EXPECT_THAT(GetAppClientStateMediumKey(L"a\\b"), EndsWith(L"a_b"));
+}
+
} // namespace updater::test
Loading diff…
Original Bug Report
reported by [email protected]
Potential arbitrary file deletion as root via path traversal in macOS GoogleUpdater
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: The system-scope GoogleUpdater on macOS computes active-bit file paths using unsanitized app IDs, allowing directory traversal. By exploiting directory ownership check flaws via /Users/Shared (which is root-owned), a local non-admin attacker can potentially bypass UID validation to delete arbitrary root-owned files. This primitive can be triggered on-demand via the unprivileged Mojo stub, leading to potential local privilege escalation.
Affected files:
chrome/updater/activity_impl_util_mac.ccchrome/updater/activity_impl_posix.ccchrome/updater/mac/privileged_helper/service.mm
Estimated timestamp from git blame: 2021-03-23
Detailed Description
A potential logic and path traversal vulnerability has been identified in the macOS system-scope (root-privileged) GoogleUpdater daemon. The updater processes ‘active-bit’ files to track user activity per application. However, because application IDs are not properly sanitized against path traversal sequences, and because /Users/Shared is treated as a valid home directory, a local attacker can potentially exploit this behavior to delete arbitrary root-owned files on the system.
1. Vulnerable Path Construction (activity_impl_util_mac.cc)
In chrome/updater/activity_impl_util_mac.cc, the function GetActiveFile constructs the file path for tracking the active state of an application using the application’s ID (id):
base::FilePath GetActiveFile(const base::FilePath& home_dir,
const std::string& id) {
return home_dir.Append("Library")
.Append(COMPANY_SHORTNAME_STRING)
.Append(KEYSTONE_NAME)
.Append("Actives")
.Append(id);
}
Because id is appended directly without validation or sanitization, if an application is registered with an ID containing path traversal components (e.g., ../../../../../../../../private/etc/sudoers.d/readme), the resulting FilePath will lexically point to a location outside the intended folder structure.
2. Root Unlink Logic and Bypass (activity_impl_posix.cc)
In chrome/updater/activity_impl_posix.cc, the ClearActiveBit function performs the deletion of the tracking file:
void ClearActiveBit(const base::FilePath& home_dir, const std::string& id) {
struct stat home_buffer = {0};
if (stat(home_dir.value().c_str(), &home_buffer)) {
return;
}
const base::FilePath active_file_path = GetActiveFile(home_dir, id);
const base::ScopedFD dir_fd(
HANDLE_EINTR(open(active_file_path.DirName().value().c_str(), O_RDONLY)));
if (!dir_fd.is_valid()) {
return;
}
struct stat active_file_buffer = {0};
if (fstatat(dir_fd.get(), active_file_path.BaseName().value().c_str(),
&active_file_buffer, AT_SYMLINK_NOFOLLOW)) {
return;
}
if (active_file_buffer.st_uid != home_buffer.st_uid) {
return;
}
unlinkat(dir_fd.get(), active_file_path.BaseName().value().c_str(), 0);
}
To prevent unauthorized file deletion, the function checks that the UID of the target file (active_file_buffer.st_uid) matches the UID of the home directory parent (home_buffer.st_uid).
However, in GetHomeDirPaths(UpdaterScope::kSystem), the updater enumerates directories under /Users and checks if they are writable. Running as root, /Users/Shared is writable. Because /Users/Shared is owned by root (UID 0), home_buffer.st_uid evaluates to 0.
If the target file (resolved via path traversal) is also owned by root, the UID check becomes 0 == 0, which evaluates to true. This allows any root-owned file to be deleted if the traversal is initiated relative to /Users/Shared.
3. Potential Trigger Mechanism
An unprivileged user can connect to the system-scope Mojo IPC service. While privileged methods like RegisterApp are blocked for untrusted callers, unprivileged callers can invoke UpdateAll or RunPeriodicTasks via the UpdateServiceStubUntrusted wrapper (defined in chrome/updater/app/server/update_service_stub.cc). These methods prompt the updater to check for updates and update metadata, which sequentially triggers GetAndClearActiveBits and invokes the vulnerable ClearActiveBit loop.
Potential Attack Steps
Note: These steps are suggested based on static code analysis; our tooling does not currently run code to provide a dynamic proof of concept.
- Directory Setup: A local unprivileged attacker creates the target directories under
/Users/Sharedto ensure the parent path resolution succeeds:mkdir -p "/Users/Shared/Library/Google/GoogleSoftwareUpdate/Actives" - App Registration: The attacker seeks to register an application ID containing the path traversal payload (e.g.,
../../../../../../../../private/etc/sudoers.d/readme). This could potentially be achieved by exploiting the SMJobBless helper’s acceptance of browser-supplied Info.plist values during a promotion race, or via other local register injection vectors. - Mojo Invocation: The attacker connects to the system-scope updater’s Mojo IPC interface as an unprivileged client and calls
UpdateAll()orRunPeriodicTasks()to trigger the sweep. - Resulting Deletion: The system daemon, executing as root, traverses from
/Users/Shared/Library/Google/GoogleSoftwareUpdate/Actives/back to/private/etc/sudoers.d/, checks that the target file is root-owned (matching/Users/Shared’s root owner), and successfully unlinks the file viaunlinkat.
Suggested Remediation
- Input Sanitization: Enforce strict validation on application IDs during registration. Reject any
app_idcontaining path separators (/), parent directory references (..), or control characters. Ideally, validateapp_idagainst a strict alphanumeric, GUID, or reverse-domain whitelist format. - Path Resolution Safety: In
activity_impl_posix.ccoractivity_impl_util_mac.cc, utilizebase::SafeBaseNameto ensure that appended application IDs cannot alter the parent directory path structure.
Evaluated with Chrome root at commit: e9507a33bb4148ee071aaaf8a7e9ad68770359bf
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.
References
On This Page